aaron.ballman added a comment.
Thanks for this! You should add test coverage for the changes, especially
around things like dependent expressions, ADL use, etc. Also, the changes need
a release note and the new functionality should be explicitly documented.
================
Comment at: clang/lib/Parse/ParsePragma.cpp:724-728
+ ExprResult E = ParseExpression();
+ if (!E.isInvalid()) {
+ E.get()->dump();
+ }
+ SkipUntil(tok::eod, StopBeforeMatch);
----------------
I think you should have another variant of `ActOnPragmaDump` that does the
actual dumping, instead of doing it from the parser. I think we should not try
to dump a dependent expression (to support those, I think we need to create a
new AST node for the pragma so that we can transform it from TreeTransform.h
and perform the dump on the non-dependent expression -- not certain if we want
to handle that now, or if we want it to be an error to use this pragma with a
dependent expression).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144115/new/
https://reviews.llvm.org/D144115
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
