[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

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

rsmith added a comment.

Thanks, this generally looks great.

It looks like we'll need some additional work on disambiguation to handle cases like:

  struct A { int n; } a;
  void f() { auto(&a)->n = 0; }

I think that's valid, but right now we misparse it as a declaration of a variable `&a`. (This syntax also resembles a function declaration with a trailing return type, but that interpretation should be rejected for various reasons, such as because `n` isn't a type.) This disambiguation failure seems to be specific to `auto`; this similar case is parsed properly:

  struct A { int n; } a;
  using T = A*;
  void f() { T(&a)->n = 1; }



================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+      // the typename-specifier in a function-style cast expression may
+      // be 'auto' since C++2b
       Diag(Tok.getLocation(),
----------------
Nice catch :)


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:1481
+  } else if (Deduced) {
+    auto Inits = Exprs;
+    if (Exprs.size() == 1) {
----------------
Please use an explicit type here.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:1482-1486
+    if (Exprs.size() == 1) {
+      if (auto p = dyn_cast_or_null<InitListExpr>(Exprs[0])) {
+        Inits = MultiExprArg(p->getInits(), p->getNumInits());
+      }
+    }
----------------
You should only do this if `ListInitialization` is `true`. Otherwise we'll accept the invalid syntax `auto({a})`.


================
Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:19
+  foo(auto {a});
+  foo(auto(a));
+
----------------
We're missing a test that we reject `auto({a})`. I assume that's what this line was intended to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393