[PATCH] D147655: Implement mangling rules for C++20 concepts and requires-expressions.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/AST/ExprConcepts.h:502
                ArrayRef<ParmVarDecl *> LocalParameters,
+               SourceLocation RParenLoc,
                ArrayRef<concepts::Requirement *> Requirements,
----------------
erichkeane wrote:
> Is this an unrelated change?  Are paren locations required for mangling?
`requires (params) { blah; }` has a different mangling from `requires { blah; }` -- in particular, the former introduces a new level of function parameters, so references to enclosing function parameters are numbered differently in the two. This applies even if `params` is empty or `void` (`requires () { ... }` and `requires (void) { ... }` are both valid).

We previously generated the same AST for `requires { ... }` and `requires () { ... }`, which meant we couldn't mangle this correctly. Tracking the parens (or at least their existence) fixes that (and also improves the AST fidelity for tooling).


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:5180
+    NotPrimaryExpr();
+    if (RE->getLParenLoc().isValid()) {
+      Out << "rQ";
----------------
erichkeane wrote:
> What is happening here? What are we deriving from the validity of the paren?  Or is this differentiating:
> 
> `requires (something)` vs `requires C<T>` sorta thing?
Yes, the `rq` form is for requires expressions without a parameter list, and the `rQ` form is for requires expressions with a parameter list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147655