This is an archive of the discontinued LLVM Phabricator instance.

testsuite: generalize `DWARFASTParserClangTests` based on `DWARFExpressionTest`'s YAML
ClosedPublic

Authored by jankratochvil on Jan 23 2020, 10:22 AM.

Details

Diff Detail

Event Timeline

jankratochvil created this revision.Jan 23 2020, 10:22 AM

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).

jankratochvil planned changes to this revision.Jan 27 2020, 5:38 AM

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

jankratochvil planned changes to this revision.Jan 30 2020, 12:57 PM

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.

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.

jankratochvil marked 2 inline comments as done.

That might explain the warnings I have seen when building with msvc the other day. It might good to add those too.

Added.

lldb/unittests/TestingSupport/CMakeLists.txt
4

Done.

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?

jankratochvil marked 8 inline comments as done.Jan 31 2020, 11:43 AM
jankratochvil added inline comments.
lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
25

some leftover, sorry

38

OK although it was even in the original file.

39

Yes, true, I did not realize that, thanks.

jankratochvil marked 3 inline comments as done.
aprantl accepted this revision.Jan 31 2020, 1:32 PM
This revision is now accepted and ready to land.Jan 31 2020, 1:32 PM
This revision was automatically updated to reflect the committed changes.