[PATCH] D144115: [clang] Extend pragma dump to support expressions

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 23 07:11:16 PST 2023


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



More information about the cfe-commits mailing list