[Clang] Fix handling of brace ellison when building deduction guides #94889

zyn0217 · 2024-06-09T03:54:52Z

Fixes two issues in two ways:

The braced-init-list consisted of initializer-list and designated-initializer-list, and thus the designated initializer is subject to [over.match.class.deduct]p1.8, which means the brace elision is also applicable on it for CTAD deduction guides.
When forming a deduction guide where the brace elision is applicable, we should also consider the presence of braces within the initializer. For example, given

template <class T, class U> struct X {
  T t[2];
  U u[3];
};

X x = {{1, 2}, 3, 4, 5};

we should establish such deduction guide AFAIU: X(T (&&)[2], U, U, U) -> X<T, U>.

Fixes #64625
Fixes #83368

llvmbot · 2024-06-10T03:53:15Z

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

Fixes two issues in two ways:

A designated-initializer-list is also a braced-init-list and thus it's subject to [over.match.class.deduct]p1.8, which means the brace elision should also apply to designated initializers for CTAD.
When forming a deduction guide with brace elision applied, we should also consider the braced initializers. For example, given

template &lt;class T&gt; struct X {
  T t[2];
};

X x = {{1, 2}};

we should establish two deduction guides AFAIU: one is for the brace-elision version i.e. X(T, T) -> X<T> and the other is for the braced version i.e. X(T (&&)[2]) -> X<T>.

Fixes #64625
Fixes #83368

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

3 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+1)
(modified) clang/lib/Sema/SemaInit.cpp (+41-15)
(modified) clang/test/SemaTemplate/deduction-guide.cpp (+70)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c700d23257bf..e5efcae2289c6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -823,6 +823,7 @@ Bug Fixes to C++ Support
   differering by their constraints when only one of these function was variadic.
 - Fix a crash when a variable is captured by a block nested inside a lambda. (Fixes #GH93625).
 - Fixed a type constraint substitution issue involving a generic lambda expression. (#GH93821)
+- Fixed two issues when building CTAD deduction guides for aggregates. (#GH64625), (#GH83368).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 79bdc8e9f8783..de2ea639bbba8 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -313,6 +313,8 @@ class InitListChecker {
   InitListExpr *FullyStructuredList = nullptr;
   NoInitExpr *DummyExpr = nullptr;
   SmallVectorImpl<QualType> *AggrDeductionCandidateParamTypes = nullptr;
+  SmallVectorImpl<QualType>
+      *AggrDeductionCandidateParamTypesWithoutBraceElision = nullptr;
 
   NoInitExpr *getDummyInit() {
     if (!DummyExpr)
@@ -506,14 +508,19 @@ class InitListChecker {
       Sema &S, const InitializedEntity &Entity, InitListExpr *IL, QualType &T,
       bool VerifyOnly, bool TreatUnavailableAsInvalid,
       bool InOverloadResolution = false,
-      SmallVectorImpl<QualType> *AggrDeductionCandidateParamTypes = nullptr);
+      SmallVectorImpl<QualType> *AggrDeductionCandidateParamTypes = nullptr,
+      SmallVectorImpl<QualType>
+          *AggrDeductionCandidateParamTypesWithoutBraceElision = nullptr);
   InitListChecker(Sema &S, const InitializedEntity &Entity, InitListExpr *IL,
                   QualType &T,
-                  SmallVectorImpl<QualType> &AggrDeductionCandidateParamTypes)
+                  SmallVectorImpl<QualType> &AggrDeductionCandidateParamTypes,
+                  SmallVectorImpl<QualType>
+                      &AggrDeductionCandidateParamTypesWithoutBraceElision)
       : InitListChecker(S, Entity, IL, T, /*VerifyOnly=*/true,
                         /*TreatUnavailableAsInvalid=*/false,
                         /*InOverloadResolution=*/false,
-                        &AggrDeductionCandidateParamTypes){};
+                        &AggrDeductionCandidateParamTypes,
+                        &AggrDeductionCandidateParamTypesWithoutBraceElision) {}
 
   bool HadError() { return hadError; }
 
@@ -982,11 +989,15 @@ static bool hasAnyDesignatedInits(const InitListExpr *IL) {
 InitListChecker::InitListChecker(
     Sema &S, const InitializedEntity &Entity, InitListExpr *IL, QualType &T,
     bool VerifyOnly, bool TreatUnavailableAsInvalid, bool InOverloadResolution,
-    SmallVectorImpl<QualType> *AggrDeductionCandidateParamTypes)
+    SmallVectorImpl<QualType> *AggrDeductionCandidateParamTypes,
+    SmallVectorImpl<QualType>
+        *AggrDeductionCandidateParamTypesWithoutBraceElision)
     : SemaRef(S), VerifyOnly(VerifyOnly),
       TreatUnavailableAsInvalid(TreatUnavailableAsInvalid),
       InOverloadResolution(InOverloadResolution),
-      AggrDeductionCandidateParamTypes(AggrDeductionCandidateParamTypes) {
+      AggrDeductionCandidateParamTypes(AggrDeductionCandidateParamTypes),
+      AggrDeductionCandidateParamTypesWithoutBraceElision(
+          AggrDeductionCandidateParamTypesWithoutBraceElision) {
   if (!VerifyOnly || hasAnyDesignatedInits(IL)) {
     FullyStructuredList =
         createInitListExpr(T, IL->getSourceRange(), IL->getNumInits());
@@ -1448,13 +1459,17 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity,
       //   brace elision is not considered for any aggregate element that has a
       //   dependent non-array type or an array type with a value-dependent
       //   bound
-      assert(AggrDeductionCandidateParamTypes);
-      if (!isa_and_nonnull<ConstantArrayType>(
+      assert(AggrDeductionCandidateParamTypes &&
+             AggrDeductionCandidateParamTypesWithoutBraceElision);
+      if (!isa_and_present<ConstantArrayType>(
               SemaRef.Context.getAsArrayType(ElemType))) {
         ++Index;
         AggrDeductionCandidateParamTypes->push_back(ElemType);
         return;
       }
+      // For array types with known bounds, we still want the brace version even
+      // though the braces can be elided.
+      AggrDeductionCandidateParamTypesWithoutBraceElision->push_back(ElemType);
     } else {
       InitializationSequence Seq(SemaRef, TmpEntity, Kind, expr,
                                  /*TopLevelOfInitList*/ true);
@@ -10918,22 +10933,24 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
       if (!(RD->getDefinition() && RD->isAggregate()))
         return;
       QualType Ty = Context.getRecordType(RD);
-      SmallVector<QualType, 8> ElementTypes;
-
-      InitListChecker CheckInitList(*this, Entity, ListInit, Ty, ElementTypes);
-      if (!CheckInitList.HadError()) {
+      auto BuildAggregateDeductionGuide = [&](MutableArrayRef<QualType>
+                                                  ElementTypes,
+                                              bool BracedVersion = false) {
+        if (ElementTypes.empty())
+          return;
         // C++ [over.match.class.deduct]p1.8:
         //   if e_i is of array type and x_i is a braced-init-list, T_i is an
         //   rvalue reference to the declared type of e_i and
         // C++ [over.match.class.deduct]p1.9:
-        //   if e_i is of array type and x_i is a bstring-literal, T_i is an
+        //   if e_i is of array type and x_i is a string-literal, T_i is an
         //   lvalue reference to the const-qualified declared type of e_i and
         // C++ [over.match.class.deduct]p1.10:
         //   otherwise, T_i is the declared type of e_i
-        for (int I = 0, E = ListInit->getNumInits();
+        for (int I = 0, E = BracedVersion ? ElementTypes.size()
+                                          : ListInit->getNumInits();
              I < E && !isa<PackExpansionType>(ElementTypes[I]); ++I)
           if (ElementTypes[I]->isArrayType()) {
-            if (isa<InitListExpr>(ListInit->getInit(I)))
+            if (isa<InitListExpr, DesignatedInitExpr>(ListInit->getInit(I)))
               ElementTypes[I] = Context.getRValueReferenceType(ElementTypes[I]);
             else if (isa<StringLiteral>(
                          ListInit->getInit(I)->IgnoreParenImpCasts()))
@@ -10951,7 +10968,16 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
                                 /*AllowAggregateDeductionCandidate=*/true);
           HasAnyDeductionGuide = true;
         }
-      }
+      };
+      SmallVector<QualType, 8> ElementTypes, ElementTypesWithoutBraceElision;
+
+      InitListChecker CheckInitList(*this, Entity, ListInit, Ty, ElementTypes,
+                                    ElementTypesWithoutBraceElision);
+      if (CheckInitList.HadError())
+        return;
+      BuildAggregateDeductionGuide(ElementTypes);
+      BuildAggregateDeductionGuide(ElementTypesWithoutBraceElision,
+                                   /*BracedVersion=*/true);
     };
 
     for (auto I = Guides.begin(), E = Guides.end(); I != E; ++I) {
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index 758ca14e4b1c3..1cce455d966df 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -335,3 +335,73 @@ namespace TTP {
 // CHECK-NEXT:      `-TemplateArgument type 'T':'type-parameter-0-0'{{$}}
 // CHECK-NEXT:        `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0{{$}}
 // CHECK-NEXT:          `-TemplateTypeParm {{.+}} 'T'{{$}}
+
+namespace GH64625 {
+
+template <class T> struct X {
+  T t[2];
+};
+
+X x = {{1, 2}}, y = {1, 2};
+
+// CHECK-LABEL: Dumping GH64625::<deduction guide for X>:
+// CHECK-NEXT: FunctionTemplateDecl {{.+}} <{{.+}}:[[#@LINE - 7]]:1, col:27> col:27 implicit <deduction guide for X>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:11, col:17> col:17 referenced class depth 0 index 0 T
+// CHECK:      |-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit <deduction guide for X> 'auto (T (&&)[2]) -> X<T>' aggregate 
+// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:27> col:27 'T (&&)[2]'
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit used <deduction guide for X> 'auto (int (&&)[2]) -> GH64625::X<int>' implicit_instantiation aggregate 
+// CHECK-NEXT:  |-TemplateArgument type 'int'
+// CHECK-NEXT:  | `-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:  `-ParmVarDecl {{.+}} <col:27> col:27 'int (&&)[2]'
+// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (T (&&)[2]) -> X<T>' dependent trailing_return
+// CHECK-NEXT: |-InjectedClassNameType {{.+}} 'X<T>' dependent
+// CHECK-NEXT: | `-CXXRecord {{.+}} 'X'
+// CHECK-NEXT: `-RValueReferenceType {{.+}} 'T (&&)[2]' dependent
+// CHECK-NEXT:  `-ConstantArrayType {{.+}} 'T[2]' dependent 2 
+// CHECK-NEXT:    `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT:      `-TemplateTypeParm {{.+}} 'T'
+
+// Brace-elision version:
+// CHECK:      |-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit <deduction guide for X> 'auto (T, T) -> X<T>' aggregate 
+// CHECK-NEXT: | |-ParmVarDecl {{.+}} <col:27> col:27 'T'
+// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:27> col:27 'T'
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit used <deduction guide for X> 'auto (int, int) -> GH64625::X<int>' implicit_instantiation aggregate 
+// CHECK-NEXT:   |-TemplateArgument type 'int'
+// CHECK-NEXT:   | `-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:   |-ParmVarDecl {{.+}} <col:27> col:27 'int'
+// CHECK-NEXT:   `-ParmVarDecl {{.+}} <col:27> col:27 'int'
+// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (T, T) -> X<T>' dependent trailing_return
+// CHECK-NEXT: |-InjectedClassNameType {{.+}} 'X<T>' dependent
+// CHECK-NEXT: | `-CXXRecord {{.+}} 'X'
+// CHECK-NEXT: |-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT: | `-TemplateTypeParm {{.+}} 'T'
+// CHECK-NEXT: `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT:  `-TemplateTypeParm {{.+}} 'T'
+
+} // namespace GH64625
+
+namespace GH83368 {
+
+template <int N> struct A {
+  int f1[N];
+};
+
+A a{.f1 = {1}};
+
+// CHECK-LABEL: Dumping GH83368::<deduction guide for A>:
+// CHECK-NEXT: FunctionTemplateDecl 0x{{.+}} <{{.+}}:[[#@LINE - 7]]:1, col:25> col:25 implicit <deduction guide for A>
+// CHECK-NEXT: |-NonTypeTemplateParmDecl {{.+}} <col:11, col:15> col:15 referenced 'int' depth 0 index 0 N
+// CHECK:      |-CXXDeductionGuideDecl {{.+}} <col:25> col:25 implicit <deduction guide for A> 'auto (int (&&)[N]) -> A<N>' aggregate
+// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:25> col:25 'int (&&)[N]'
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:25> col:25 implicit used <deduction guide for A> 'auto (int (&&)[1]) -> GH83368::A<1>' implicit_instantiation aggregate 
+// CHECK-NEXT:   |-TemplateArgument integral '1'
+// CHECK-NEXT:   `-ParmVarDecl {{.+}} <col:25> col:25 'int (&&)[1]'
+// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (int (&&)[N]) -> A<N>' dependent trailing_return
+// CHECK-NEXT: |-InjectedClassNameType {{.+}} 'A<N>' dependent
+// CHECK-NEXT: | `-CXXRecord {{.+}} 'A'
+// CHECK-NEXT: `-RValueReferenceType {{.+}} 'int (&&)[N]' dependent
+// CHECK-NEXT:   `-DependentSizedArrayType {{.+}} 'int[N]' dependent
+// CHECK-NEXT:     |-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:     `-DeclRefExpr {{.+}} <col:10> 'int' NonTypeTemplateParm {{.+}} 'N' 'int'
+
+} // namespace GH83368

cor3ntin

I think the commit message / release notes message need work (" Fix handling of brace ellison when building deduction guides",

But the general direction look good

clang/lib/Sema/SemaInit.cpp

clang/test/SemaTemplate/deduction-guide.cpp

zyn0217 · 2024-06-12T06:48:20Z

@hokein I realized the entire patch could be significantly simplified, in which we just don't need to perform the brace elision on a braced initializer. Can you take a second look at it?

clang/lib/Sema/SemaInit.cpp

cor3ntin

LGTM

clang/lib/Sema/SemaInit.cpp

zyn0217 · 2024-06-13T08:40:59Z

Thanks for the review. :)

…lvm#94889) Fixes two issues in two ways: 1) The `braced-init-list` consisted of `initializer-list` and `designated-initializer-list`, and thus the designated initializer is subject to [over.match.class.deduct]p1.8, which means the brace elision is also applicable on it for CTAD deduction guides. 2) When forming a deduction guide where the brace elision is applicable, we should also consider the presence of braces within the initializer. For example, given template <class T, class U> struct X { T t[2]; U u[3]; }; X x = {{1, 2}, 3, 4, 5}; we should establish such deduction guide AFAIU: `X(T (&&)[2], U, U, U) -> X<T, U>`. Fixes llvm#64625 Fixes llvm#83368

zyn0217 added 2 commits June 9, 2024 11:53

[Clang] Fix two issues of CTAD for aggregates

217c00f

Release notes & AST verification.

6c723e8

zyn0217 requested review from mizvekov, hokein and erichkeane June 10, 2024 03:52

zyn0217 marked this pull request as ready for review June 10, 2024 03:52

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 10, 2024

cor3ntin reviewed Jun 10, 2024

View reviewed changes

clang/lib/Sema/SemaInit.cpp Outdated Show resolved Hide resolved

zyn0217 changed the title ~~[Clang] Fix two issues of CTAD for aggregates~~ [Clang] Fix handling of brace ellison when building deduction guides Jun 10, 2024

Apply Corentin's feedback

da3df40

hokein reviewed Jun 11, 2024

View reviewed changes

zyn0217 added 2 commits June 11, 2024 23:29

Fix handling of two or more aggregates

c644f10

Simplify the whole things

16cf879

Tidy up the comment

b3dd755

hokein reviewed Jun 12, 2024

View reviewed changes

clang/lib/Sema/SemaInit.cpp Show resolved Hide resolved

clang/lib/Sema/SemaInit.cpp Show resolved Hide resolved

Clarify the comments

7a70867

zyn0217 requested a review from hokein June 13, 2024 03:41

cor3ntin approved these changes Jun 13, 2024

View reviewed changes

clang/lib/Sema/SemaInit.cpp Outdated Show resolved Hide resolved

zyn0217 added 2 commits June 13, 2024 16:37

Address comments

1477377

Merge branch 'main' into aggregate-ctad-issues

2ceaa2a

zyn0217 merged commit a144bf2 into llvm:main Jun 13, 2024
6 of 7 checks passed

zyn0217 deleted the aggregate-ctad-issues branch June 13, 2024 09:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Fix handling of brace ellison when building deduction guides #94889

[Clang] Fix handling of brace ellison when building deduction guides #94889

Uh oh!

zyn0217 commented Jun 9, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jun 10, 2024

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zyn0217 commented Jun 12, 2024

Uh oh!

Uh oh!

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

zyn0217 commented Jun 13, 2024

Uh oh!

Uh oh!

Uh oh!

[Clang] Fix handling of brace ellison when building deduction guides #94889

[Clang] Fix handling of brace ellison when building deduction guides #94889

Uh oh!

Conversation

zyn0217 commented Jun 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 10, 2024

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zyn0217 commented Jun 12, 2024

Uh oh!

Uh oh!

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zyn0217 commented Jun 13, 2024

Uh oh!

Uh oh!

Uh oh!

zyn0217 commented Jun 9, 2024 •

edited

Loading