[clang] Refine handling of C++20 aggregate initialization

[offsetof]

• Move parts of InitializationSequence::InitializeFrom corresponding to C++ [dcl.init.general] p16.6.1 and p16.6.2 into a separate function,TryConstructorOrParenListInitialization
• Use it in TryListInitialization to implement [dcl.init.list] p3.2
• Fix parenthesized aggregate initialization being attempted in copy-initialization contexts or when the constructor call is ambiguous

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (offsetof)

Changes

Fixes #88089

---

Full diff: https://github.com/llvm/llvm-project/pull/131320.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaInit.cpp (+2-1)
• (modified) clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp (+29-13)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 56ec33fe37bf3..11db8608960ce 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6714,7 +6714,8 @@ void InitializationSequence::InitializeFrom(Sema &S,
         OverloadCandidateSet::iterator Best;
         OverloadingResult OR = getFailedCandidateSet().BestViableFunction(
             S, Kind.getLocation(), Best);
-        if (OR != OverloadingResult::OR_Deleted) {
+        if (OR != OverloadingResult::OR_Deleted &&
+            Kind.getKind() == InitializationKind::IK_Direct) {
           // C++20 [dcl.init] 17.6.2.2:
           //   - Otherwise, if no constructor is viable, the destination type is
           //   an
diff --git a/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp b/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
index 1ad205d769c38..1ef74bb66b4de 100644
--- a/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
@@ -1,18 +1,34 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-// If the initializer is (), the object is value-initialized.
-
-// expected-no-diagnostics
 namespace GH69890 {
-struct A {
-    constexpr A() {}
-    int x;
-};
+    // If the initializer is (), the object is value-initialized.
+    struct A {
+        constexpr A() {}
+        int x;
+    };
+
+    struct B : A {
+        int y;
+    };
+
+    static_assert(B().x == 0);
+    static_assert(B().y == 0);
+} // namespace GH69890
+
+namespace P0960R3 {
+    struct A { // expected-note 9 {{candidate constructor}}
+        int i;
+        operator int() volatile;
+    };
+
+    volatile A va;
+    A a = va; // expected-error {{no matching constructor for initialization of 'A'}}
 
-struct B : A {
-    int y;
-};
+    A f() {
+        return va; // expected-error {{no matching constructor for initialization of 'A'}}
+    }
 
-static_assert(B().x == 0);
-static_assert(B().y == 0);
-}
+    int g(A); // expected-note {{passing argument to parameter here}}
+    int g(auto&&);
+    int i = g(va); // expected-error {{no matching constructor for initialization of 'A'}}
+} // namespace P0960R3

[cor3ntin]

I don't think this resolves the original issue.
Part of the problem is that we are missing a diagnostic saying the implicit constructor is deleted (and why).

[cor3ntin]

Presumably, this is https://eel.is/c++draft/dcl.init.general#16.6.2.sentence-1
Why do you think copy should not be valid here?

Afaik, the core issue @frederick-vs-ja mentioned here, #88089 (comment), only applies to the array case.

[offsetof]

This is https://eel.is/c++draft/dcl.init.general#16.6.2.2.sentence-1, specifically "and the initializer is a parenthesized expression-list". The bug is that X x = y; is treated as X x(y); when y has type cv X and X has no viable constructor for the initialization. (I should have explained that in the OP; sorry.)
The linked core issue is indeed related to arrays (handled by a different bullet) and is not relevant here.

[cor3ntin]

Oh, I missed that. Thanks for the explanation.

Can you

• Add that wording sentence as a comment
• Add tests demonstrating the parent-init case and brace-init cases?

[offsetof]

I discovered a couple more issues in this area, and updated the PR with a more comprehensive fix, along with more tests and a release note as requested.

[offsetof]

I don't think this resolves the original issue.
Part of the problem is that we are missing a diagnostic saying the implicit constructor is deleted (and why).

Fair enough; I linked that issue because the nonsensical diagnostic mentioned there ("no viable conversion from S to int") is a side-effect of the bug fixed by this PR. With the change applied, we get the usual "no matching constructor" with a list of candidates. But if we want something else, then the issue should remain open.

[cor3ntin]

This change needs a release note.
Please add an entry to clang/docs/ReleaseNotes.rst in the section the most adapted to the change, and referencing any Github issue this change fixes. Thanks!

[cor3ntin]

Oh, I missed that. Thanks for the explanation.

Can you

• Add that wording sentence as a comment
• Add tests demonstrating the parent-init case and brace-init cases?

[cor3ntin]

@offsetof CI got confused. Can you try to push an empty commit? Sorry about that

[offsetof]

Seems to be working now, I think.

[cor3ntin]

Thanks!

[cor3ntin]

@offsetof Will you need me to merge that for you?

[offsetof]

That would be great. Thank you!