[PATCH] D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>.

James Y Knight via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 27 06:43:12 PST 2021


jyknight added inline comments.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3912
+    IsPrimaryExpr = false;
+  };
+
----------------
rjmccall wrote:
> I think it might be more reasonable to just check for the relatively small number of primary-expression cases in `mangleTemplateArgExpr` and skip the `X...E` there rather than pushing it down into this function.
I started out writing it that way, but it's more complex than it first seems.

First you have the 9 expression types that always fit into <expr-primary> -- those are simple enough, but that's already a lot more than one might think. But, then you also need to handle recusing on all the cases with no output (ParenExprClass etc. -- 10 cases of this). And, finally, you have complex cases like DeclRefExpr, CXXConstructExprClass, and UnaryExprOrTypeTraitExprClass, which sometimes emit a primary expression and sometimes do not.

Putting all of that together, the number of cases to handle with a separate function was large enough -- and duplicative enough -- that it seemed more confusing to have such an implementation split than it was helpful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95487/new/

https://reviews.llvm.org/D95487



More information about the cfe-commits mailing list