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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
