[PATCH] D129531: [clang][C++20] P0960R3: Allow initializing aggregates from a parenthesized list of values

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

It starting to look great.
Should we add an extension warning? Note that I'm not suggesting to support that in earlier modes, just an off-by-default warning that says "this is a c++20 feature". but we are a bit inconsistent with those.

can you rename `test/SemaCXX/P0960R3.cpp` to something like `test/SemaCXX/cxx20-paren-list-init.cpp` (and same for other tests?) it's more consistent with other existing tests.



================
Comment at: clang/include/clang/Sema/Sema.h:6200
 
-
   /// Instantiate or parse a C++ default argument expression as necessary.
----------------
Whitespace change


================
Comment at: clang/lib/Sema/SemaInit.cpp:3528
     delete ICS;
+    break;
   }
----------------
It seems unrelated. but reasonable.


================
Comment at: clang/lib/Sema/SemaInit.cpp:5379-5380
+    } else {
+      Sequence.SetFailed(
+          InitializationSequence::FK_ParenthesizedListInitFailed);
+    }
----------------
Shouldn't we do that unconditionally?


================
Comment at: clang/lib/Sema/SemaInit.cpp:6106
+
+      // We fall back to the "no matching constructor "path iff the
+      // failed candidate set has function other than the three default
----------------



================
Comment at: clang/lib/Sema/SemaInit.cpp:6107
+      // We fall back to the "no matching constructor "path iff the
+      // failed candidate set has function other than the three default
+      // constructors. For example, conversion function.
----------------



================
Comment at: clang/lib/Sema/SemaInit.cpp:6114-6116
+        // const ValueDecl *VD = Entity.getDecl();
+        // if (const VarDecl *VarD = dyn_cast_or_null<VarDecl>(VD);
+        //     VarD && VarD->hasInit() && !VarD->getInit()->containsErrors())
----------------
Is that code meant to be commented?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531