[PATCH] D125479: [pseudo] Fix the incorrect parameters-and-qualifiers rule.

[Sam McCall via Phabricator via cfe-commits]

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