This is an archive of the discontinued LLVM Phabricator instance.

[Testing] TestAST, a helper for writing straight-line AST tests
ClosedPublic

Authored by sammccall on Apr 13 2022, 4:10 AM.

Details

Summary

Tests that need ASTs have to deal with the awkward control flow of
FrontendAction in some way. There are a few idioms used:

  • don't bother with unit tests, use clang -dump-ast
  • create an ASTConsumer by hand, which is bulky
  • use ASTMatchFinder - works pretty well if matchers are actually needed, very strange if they are not
  • use ASTUnit - this yields nice straight-line code, but ASTUnit is a terrifically complicated library not designed for this purpose

TestAST provides a very simple way to write straight-line tests: specify
the code/flags and it provides an AST that is kept alive until the
object is destroyed.
It's loosely modeled after TestTU in clangd, which we've successfully
used for a variety of tests.

I've updated a couple of clang tests to use this helper, IMO they're clearer.

Diff Detail

Event Timeline

sammccall created this revision.Apr 13 2022, 4:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 4:10 AM
sammccall requested review of this revision.Apr 13 2022, 4:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 4:10 AM
nridge added a subscriber: nridge.Apr 17 2022, 4:04 PM
kbobyrev accepted this revision.Apr 20 2022, 5:14 AM

Thanks, this looks good; just few nits regarding the comments.

clang/include/clang/Testing/TestAST.h
10

nit: capitalization

34

nit: omit TestInputs since it's the document for that.

55
71
clang/lib/Testing/TestAST.cpp
2

nit: comment seems broken

This revision is now accepted and ready to land.Apr 20 2022, 5:14 AM
This revision was landed with ongoing or failed builds.Apr 21 2022, 12:46 PM
This revision was automatically updated to reflect the committed changes.
sammccall marked 5 inline comments as done.
thakis added a subscriber: thakis.Apr 21 2022, 6:30 PM
thakis added inline comments.
clang/lib/Testing/CMakeLists.txt
18

Making clang/lib/Testing depend on clangFrontend (and hence on basically everything) seems pretty heavy. Maybe this should be in a new library?

(In general, be wary when you write a CL desc that goes "people currently do X in the following N ways, so let's add a N+1th way". This feels short-term good for you since the N+1th way is written exactly like you'd write it – you wrote it, after all! But for everyone else, N went up.)

sammccall marked an inline comment as done.Apr 22 2022, 1:13 AM
sammccall added a subscriber: hliao.

It looks like @hliao fixed these aready, thanks!

clang/lib/Testing/CMakeLists.txt
18

I don't think there's any real dependency regression here. AFAICS everything using clangTesting was also using Frontend already, and this isn't a coincidence.

Before this patch, clang/lib/testing contained only lists of command-line flags. In practice these are useful for parsing code in a variety of configurations, which requires frontend and everything.

(In general I *would* prefer to have many fine-grained libraries of a header or two each, but our CMake setup makes it very costly to do so)