Glow: Failure when building tests (gmock)

[tlepley-cadence]

I'm having issues compiling tests. It started few weeks ago.
I tried to do a clean rebuild from scratch, and I get the same result.

Any idea what's wrong ?

Compiler(s):
It failed with both gcc 8.1 and gcc 8.2. I tried with clang 7.0 and it fails somewhere else.
gcc 8.2 is: gcc version 8.2.1 20181011 (Red Hat 8.2.1-4) (GCC)

CMake command:
cmake -G Ninja -DCMAKE_PREFIX_PATH=/home/tlepley/pub -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DGLOW_WITH_CPU=ON -DGLOW_WITH_OPENCL=OFF -DGLOW_BUILD_EXAMPLES=ON -DGLOW_BUILD_TESTS=ON ../glow

Failing gcc command:
/usr/bin/g++ -DGLOW_WITH_CPU=1 -I/home/tlepley/git/glow/include -Iinclude -I. -isystem /home/tlepley/pub/include -I/home/tlepley/git/glow/thirdparty/fp16/include -I/home/tlepley/git/glow/tests/googletest/googlemock/include -I/home/tlepley/git/glow/tests/googletest/googlemock -isystem /home/tlepley/git/glow/tests/googletest/googletest/include -isystem /home/tlepley/git/glow/tests/googletest/googletest -Wall -Wnon-virtual-dtor -fno-exceptions -fno-rtti -g -fno-omit-frame-pointer -O0 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -std=c++11 -Wall -Wshadow -Werror -Wno-error=dangling-else -DGTEST_HAS_PTHREAD=1 -fexceptions -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -pthread -std=c++14 -MD -MT tests/googletest/googlemock/CMakeFiles/gmock.dir/src/gmock-all.cc.o -MF tests/googletest/googlemock/CMakeFiles/gmock.dir/src/gmock-all.cc.o.d -o tests/googletest/googlemock/CMakeFiles/gmock.dir/src/gmock-all.cc.o -c /home/tlepley/git/glow/tests/googletest/googlemock/src/gmock-all.cc

Sample of error messages (there are too many):

In file included from /home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-spec-builders.h:71,
                 from /home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-generated-function-mockers.h:44,
                 from /home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock.h:62,
                 from /home/tlepley/git/glow/tests/googletest/googlemock/src/gmock-all.cc:39:
/home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-matchers.h:334:22: error: 'IsSame' is not a member of 'testing::internal'
           !internal::IsSame<U, GTEST_REFERENCE_TO_CONST_(U)>::value>::type* =
                      ^~~~~~
/home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-matchers.h:334:22: note: suggested alternative: 'IsSpace'
           !internal::IsSame<U, GTEST_REFERENCE_TO_CONST_(U)>::value>::type* =
                      ^~~~~~
                      IsSpace
/home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-matchers.h:334:28: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses]
           !internal::IsSame<U, GTEST_REFERENCE_TO_CONST_(U)>::value>::type* =
                            ^
/home/tlepley/git/glow/tests/googletest/googlemock/include/gmock/gmock-matchers.h:334:60: error: wrong number of template arguments (2, should be 1)
           !internal::IsSame<U, GTEST_REFERENCE_TO_CONST_(U)>::value>::type* =

[qcolombet]

Hmm, that's strange, this specific construction is supposed to be defined in gtest/internal/gtest-port.h, which is included in gmock/internal/gmock-port.h, which is itself include in gmock/gmock-matchers.h. Therefore this file should have no issue with that specific definition.

I guess you've already tried, but if not, could you remove the googletest directory and "git submodule update" it again?

If that doesn't work, look at the preprocessed file to see where the expansions of the includes come from.

[bertmaher]

Wouldn't surprise me if our submodule bump of googletest landed on a revision that's broken for your compiler. You could try pulling latest master to see if that works: (cd tests/googletest/ && git fetch && git checkout origin/master)

[tlepley-cadence]

I think I figured out the problem. I had gmock and gtestincludes installed in my system.
By using -DCMAKE_PREFIX_PATH=/home/tlepley/pub in the cmake command line, I get -isystem /home/tlepley/pub/include in the g++ command line before the gtest related -I options. The system gmock/gtest headers then get included instead of the headers of the glow directory. This is what created the compilation failure.

It think it would make sense to ensure that glow internal headers directories are specified before any system directory in the g++ command line. With this, we would avoid problem when any of the third party code used by glow is already installed in the system.

Any opinion ?

[qcolombet]

Have you checked that fixes the problem?

I indeed would have hoped that given the includes are made with "" (instead of <>) gcc would look in user defined directories before looking in the system ones.

Checking the standard, the search order is implementation defined anyway, so all bets are off :(.

[tlepley-cadence]

@qcolombet for now, I simply moved the 'system' header files and checked that it compiles. I'm now going to experiment changes in the cmake files to put system includes later in the g++ command line. If it's successful, I'll propose a PR.

[qcolombet]

May we close this?