sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/pseudo/test/glr.cpp:37 + +void foo2(int, ...); +// CHECK: declaration~simple-declaration := decl-specifier-seq init-declarator-list ; ---------------- hokein wrote: > since we have the builtin pseudoCXX library now, we could use it to write a > cxx unittest, so that we can get rid of this cxx lit test. what do you think? > (I have a feeling we might add more in the future, I just discovered a new > oversight in our cxx.bnf grammar). For detailed coverage of particular parts, t's not obvious to me whether a unit test or these tests will be more readable. (Consider writing a bunch of matchers and dealing with the messages from those, vs textual diffs here). ================ Comment at: clang-tools-extra/pseudo/test/glr.cpp:37 + +void foo2(int, ...); +// CHECK: declaration~simple-declaration := decl-specifier-seq init-declarator-list ; ---------------- sammccall wrote: > hokein wrote: > > since we have the builtin pseudoCXX library now, we could use it to write a > > cxx unittest, so that we can get rid of this cxx lit test. what do you > > think? (I have a feeling we might add more in the future, I just discovered > > a new oversight in our cxx.bnf grammar). > For detailed coverage of particular parts, t's not obvious to me whether a > unit test or these tests will be more readable. (Consider writing a bunch of > matchers and dealing with the messages from those, vs textual diffs here). can we place this test in another file? (glr/varargs.cpp or so) I missed this with `operator<` but that should probably be its own file too. One of the things that feels painful with clang's lit tests is having to reduce a failure in a huge file with many related tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125479/new/ https://reviews.llvm.org/D125479 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
