Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks for splitting this off.
This looks good except that I wouldn't want to put this under the base TestingSupport folder -- the idea there was that this would contain only the basic stuff with no fancy dependencies (so that e.g. UtilityTests don't depend on everything). Here, you've put everything inline, which kind of hides this, but now that this is a separate file, it would make sense to put some of this stuff into a .cpp file (e.g. YamlObjectFile, YamlModule classes, and the YAMLModuleTester constructor).
The exact layout of the folders under TestingSupport is still evolving, and although one could argue that this should go under TestingSupport/SymbolFile/DWARF, that is quite a mouthful, and so I think we can just shove this under the existing TestingSupport/Symbol folder (with the rationale that SymbolFile, base class of SymbolFileDWARF, lives in the Symbol folder).
By making the new .cpp file I get now this error:
In file included from /home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp:9: In file included from /home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h:14: In file included from /home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/SubsystemRAII.h:13: In file included from /home/jkratoch/redhat/llvm-monorepo/llvm/include/llvm/Testing/Support/Error.h:14: /home/jkratoch/redhat/llvm-monorepo/llvm/include/llvm/Testing/Support/SupportHelpers.h:16:10: fatal error: 'gmock/gmock-matchers.h' file not found #include "gmock/gmock-matchers.h" ^~~~~~~~~~~~~~~~~~~~~~~~
If anyone has an idea... Originally the header file was being built by: lldb/unittests/Expression/CMakeLists.txt
It needed to add the line:
include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)
But then maybe it also needs:
# Our current version of gtest does not properly recognize C++11 support # with MSVC, so it falls back to tr1 / experimental classes. Since LLVM # itself requires C++11, we can safely force it on unconditionally so that # we don't have to fight with the buggy gtest check. add_definitions(-DGTEST_LANG_CXX11=1) add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
as being used in such cases in cmakefiles in llvm/ but on Linux I do not see if it is really needed here or not.
That might explain the warnings I have seen when building with msvc the other day. It might good to add those too.
lldb/unittests/TestingSupport/CMakeLists.txt | ||
---|---|---|
4 | The idea was then that this would be a separate library (lldbSymbolHelpers) with its own CMakeLists.txt and everything. |
I like the direction!
lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp | ||
---|---|---|
2 | Nit: the -*- C++ -*- only makes sense in a .h file where the language is ambiguous. | |
lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h | ||
25 | ? | |
38 | /// | |
39 | Should this be implemented in a sub-class that is specific to the DWARFExpression tests? |
The idea was then that this would be a separate library (lldbSymbolHelpers) with its own CMakeLists.txt and everything.