[Clang] Implements CTAD for aggregates P1816R0 and P2082R1

[ychen]

format

[cor3ntin]

Hey. Thanks a lot for working on this.
I did a first pass, looking at mostly style issues, looking at conformance will probably take me a lot more time, but i think this looks pretty good overall

clang/include/clang/AST/DeclCXX.h

1951

I'm wondering if the constructor should take a DeductionCandidateKind defaulted to normal here. All the places where it's set seem to be immediately after construction.

1993–1997

I'm not sure how useful these things are, isAggregateDeductionCandidate is only used once

clang/lib/Sema/SemaInit.cpp

39

This does not seems used

314

I think using optional here would make more sense, I guess that's why you included it.
Should it be SmallVectorImpl, such that only the caller has to bake in a size?

1103–1104

Can you had a comment about that case? I'm not sure i understand what scenario we are handling

2187–2189

Maybe using enumerate + a range for loop here would be cleaner?

clang/lib/Sema/SemaOverload.cpp

7265–7266

[ychen]

• Address Corentin's comments

[ychen]

In D139837#3989814, @cor3ntin wrote:

Hey. Thanks a lot for working on this.
I did a first pass, looking at mostly style issues, looking at conformance will probably take me a lot more time, but i think this looks pretty good overall

Thanks for the quick fist-pass review. Appreciate it.

clang/include/clang/AST/DeclCXX.h

1951

That's true indeed. The awkward aspect is that the CXXDeductionGuideDecl::Create call is far from setDeductionCandidateKind; making CXXDeductionGuideDecl::Create takes a DeductionCandidateKind would several other less related functions takes DeductionCandidateKind also.

1993–1997

I meant to make it consistent with isCopyDeductionCandidate which is also used once. Maybe remove both isCopyDeductionCandidate and isAggregateDeductionCandidate?

clang/lib/Sema/SemaInit.cpp

314

I meant to make AggrDeductionCandidateParamTypes a byref semantic like FullyStructuredList. So the InitListChecker populates them as a side-effect. Using optional means the InitListChecker owes the deduced types which should work too but seems weird to me.

1103–1104

Good point. On second thought, I think this is not needed. This function shouldn't accept dependent types.

2187–2189

Using the iterator to test for the last element is easier. The index produced by enumerate would need to produce an iterator nevertheless.

[ychen]

• update cxx_status.html

[cor3ntin]

Sorry for not getting back to you earlier.
It mostly looks reasonable to me.
what do you think @erichkeane ?

clang/lib/Sema/SemaInit.cpp

10740

I think we need to support A{("Hello")}; so we probably need to get rid of parentheses (maybe other implicit nodes?)

10766

Maybe add a message here

clang/lib/Sema/SemaTemplate.cpp

2596–2597

Maybe we should have an asserton !isInvalid then?

clang/lib/Sema/SemaTemplateDeduction.cpp

363

I think you can do that in the for loop as it was before

995

That could always be initialized, to avoid future surprises

[cor3ntin]

clang/include/clang/AST/DeclCXX.h

1993–1997

Yes, i think we might as well

[cor3ntin]

+ @hubert.reinterpretcast In case i missed something conformance wise.

Speaking of conformance, your implementation does not appear to gate the feature on C++20, which it should, so you should add a test that it doesn't compile in c++17 (I'm not sure we could support it as an extension... maybe?)

[cor3ntin]

clang/lib/Sema/SemaInit.cpp

13

Do we actually need that?

[shafik]

clang/lib/Sema/SemaInit.cpp

10676–10680

Minor issues, we need to match the format expected by bugprone-comment

clang/lib/Sema/SemaOverload.cpp

7386–7387

formatting fix again.

clang/lib/Sema/SemaTemplate.cpp

2593

formatting.

[ayzhao]

Thanks for your effort on this!

FYI I landed parenthesized aggregate initialization in D141546, so CTAD for that feature should no longer be blocked.

[ychen]

In D139837#4067208, @ayzhao wrote:

Thanks for your effort on this!

FYI I landed parenthesized aggregate initialization in D141546, so CTAD for that feature should no longer be blocked.

Thanks for letting me know. Considering the existing reviews, I'll create a separate patch for the parenthesized expression-list support.

[cor3ntin]

@ychen Hey! Are you still working on this? Thanks!

[cor3ntin]

@erichkeane @aaron.ballman I think we should determine how much works remains there and if the author is not responsive maybe one of us can push this up the finishing line.

[aaron.ballman]

In D139837#4326501, @cor3ntin wrote:

@erichkeane @aaron.ballman I think we should determine how much works remains there and if the author is not responsive maybe one of us can push this up the finishing line.

That seems like a reasonable approach to me; it'd be good to get this into Clang 17 if possible, and @ychen seems to have been away for ~2 months now, so it seems reasonable to commandeer this review if we need to.

[ychen]

Sorry for the long delay guys. I'll update the patch this weekend.

[ychen]

clang/lib/Sema/SemaInit.cpp

10740

Agreed. Fixed.

clang/lib/Sema/SemaTemplate.cpp

2596–2597

Looked at this a little bit more. It can fail (D46446). So I updated the comments in a few places.

clang/lib/Sema/SemaTemplateDeduction.cpp

363

Done

[ychen]

Address all comments

[ychen]

rebase

[ychen]

• add release notes

[cor3ntin]

I think this is starting to look good, My biggest question is the remaining fixme, how much work would it be to do in this PR?

clang/include/clang/Sema/TemplateDeduction.h

237 ↗

(On Diff #521965)

Can you specify which c++ version this quote is from?

clang/lib/Sema/SemaTemplate.cpp

2597

Maybe you could do this in this PR?

clang/www/cxx_status.html

1249 ↗

(On Diff #521965)

I think this should be marked a partially supported until we support parenthesized init.

[ychen]

• Fix bitfield on Windows

[ychen]

@cor3ntin Thanks for the review. I've added the parenthesized expression-list support by treating it as braced-init-list during CTAD.

• adjust test check for Windows
• support parenthesized expression-list
• add an InitializationKind BuildingDeductionGuides to help diagnoses

[cor3ntin]

Thanks, It's great to have the complete feature in on PR

clang/include/clang/Sema/Sema.h

9661 ↗

(On Diff #524045)

Is that comment correct?

clang/lib/Sema/SemaDecl.cpp

12637 ↗

(On Diff #524045)

clang/lib/Sema/SemaTemplateInstantiate.cpp

1059–1062 ↗

(On Diff #524045)

This seems unfinished

[aaron.ballman]

Thank you for working on this! Some high-level comments while I was here; I'm still grokking the implementation.

clang/include/clang/AST/DeclBase.h

1689 ↗

(On Diff #524045)

Best not to give this the same name as a type (I don't care which one changes names).

clang/include/clang/AST/DeclCXX.h

1986

Er, seems a bit odd to cast an 8-bit type to a 64-bit type only to shove it into a 2-bit bit-field. I think DeductionCandidateKind should be an enum class whose underlying type is int and we cast to/from int as needed.

clang/include/clang/Sema/Sema.h

3992 ↗

(On Diff #524045)

We're up to 12 parameters for this function, five of which are bool parameters... at some point, this probably needs to be refactored.

9346 ↗

(On Diff #524045)

[aaron.ballman]

clang/include/clang/Sema/Sema.h

9661 ↗

(On Diff #524045)

Yeah, the comment seems off to me.

clang/lib/Sema/SemaInit.cpp

504–515

We shouldn't force the caller to use the same-sized SmallVector, right?

1036

Can we make this return a const RecordDecl * or does that run into viral const issues?

1037–1038

1039–1040

1444–1446

Be sure to add test coverage for use of VLAs in C++ (we support it as an extension).

10699

10710–10711

clang/lib/Sema/SemaTemplate.cpp

2576

Something is amiss here. Either this should be using dyn_cast or it should not be in an if statement (cast cannot fail; it asserts if it does).

clang/lib/Sema/SemaTemplateInstantiate.cpp

1059–1062 ↗

(On Diff #524045)

+1

[ychen]

• address existing comments

clang/include/clang/AST/DeclBase.h

1689 ↗

(On Diff #524045)

I've changed the type name and kept this as is.

clang/include/clang/Sema/Sema.h

3992 ↗

(On Diff #524045)

Agreed. I will keep my eye on it.

clang/lib/Sema/SemaInit.cpp

504–515

That's right.

1036

"viral const issues". Deep somewhere else needs it non-const.

1444–1446

Array::a2 test case covers this.

clang/lib/Sema/SemaTemplate.cpp

2576

It's the getDefinition() that may be null. I've hoist the cast out to make it obvious.

clang/lib/Sema/SemaTemplateInstantiate.cpp

1059–1062 ↗

(On Diff #524045)

I meant to keep this a future work since this path is dead until some errors could be thrown out of this context. In the future, if errors happen during building deduction guides, this assertion failure could trigger at build time.

[cor3ntin]

clang/lib/Sema/SemaTemplateInstantiate.cpp

1059–1062 ↗

(On Diff #524045)

In this case, maybe change the message
assert(false && "unexpected deduction guide in instantiation stack") or something along those lines.

[ychen]

• address comment

[ychen]

clang/lib/Sema/SemaTemplateInstantiate.cpp

1059–1062 ↗

(On Diff #524045)

Done. I've made it llvm_unreachable to catch the rare chances that users hit this.

[cor3ntin]

I think this looks good but I'll let @aaron.ballman do the final approval.

[ychen]

In D139837#4401134, @cor3ntin wrote:

I think this looks good but I'll let @aaron.ballman do the final approval.

Thanks for the review @cor3ntin.

[shafik]

clang/test/SemaTemplate/aggregate-deduction-candidate.cpp

101 ↗

(On Diff #528918)

I would also like to see this test:

template <typename T>
struct I {
  using type = T;
};

template <typename T>
struct E {
  typename I<T>::type i;
  T t;
};

E e1 = {1, 2}; // OK, E<int> deduced

Since it is in the paper, this should cover it but it can't hurt.

[ychen]

• add one extra test

[ychen]

clang/test/SemaTemplate/aggregate-deduction-candidate.cpp

101 ↗

(On Diff #528918)

Yep, just added this test.

[shafik]

clang/include/clang/AST/DeclBase.h

1686 ↗

(On Diff #528918)

Why remove [C++17] ?

clang/include/clang/AST/DeclCXX.h

1986

It feels a bit weird that we are taking an enum w/ an underlying type of unsigned char casting it to int and then placing it in an unsigned bit-field. I don't have a better suggestion ATM but I wish we had something better.

clang/lib/Sema/SemaDecl.cpp

12647 ↗

(On Diff #528918)

nitpick but we seem to use Expr* everywhere else.

clang/lib/Sema/SemaInit.cpp

15

I saw your adding headers. How did you figure out which ones were missing?

508

nit

10680

nit,

note: PO` is not a very descriptive name.

10706–10707

Quoted Text

10734

10741

[cor3ntin]

clang/include/clang/AST/DeclBase.h

1686 ↗

(On Diff #528918)

I don't think this adds any information, I'm happy to see it removed. And arguably in c++17 there was only one kind of DG.

[ychen]

• address comments

clang/include/clang/AST/DeclCXX.h

1986

I've changed it to casting to underlying type of DeductionCandidate so it has one less step of conversion.

clang/lib/Sema/SemaInit.cpp

15

Added headers provide APIs for the new code. I guess many of them are included already indirectly (SmallVector for example). But I thought the best practice is not relying on that, so I added these.

[cor3ntin]

clang/lib/Sema/SemaInit.cpp

15

I think it's easier fore reviewers not to add things are are not needed, I would suggest trying to keep the change to the list of includes to the minimum

[ychen]

• remove extra headers

[cor3ntin]

@ychen You probably want to update the description, i think it's updated (you implemented ctad for parenthesized ctrs)

[ychen]

In D139837#4449327, @cor3ntin wrote:

@ychen You probably want to update the description, i think it's updated (you implemented ctad for parenthesized ctrs)

Thanks for the remainder, Corentin. Hopefully, this will be ready to be landed soon.

[cor3ntin]

In D139837#4449536, @ychen wrote:

In D139837#4449327, @cor3ntin wrote:

@ychen You probably want to update the description, i think it's updated (you implemented ctad for parenthesized ctrs)

Thanks for the remainder, Corentin. Hopefully, this will be ready to be landed soon.

I hope so too. I pinged @aaron.ballman :)

[erichkeane]

Thanks for this! I have no concerns from what I can see, but I see the others are doing a very thorough review, so I'll let them do the approvals.

clang/include/clang/AST/DeclCXX.h

1951

I'm in favor of having this be a part of CXXDeductionGuideDecl::Create and the ctor as a parameter instead as well.

[ychen]

• add a DeductionCandidate parameter

[cor3ntin]

clang/include/clang/AST/DeclBase.h

1687 ↗

(On Diff #534778)

clang/include/clang/AST/DeclCXX.h

1993–1997

I still think we should remove that

clang/include/clang/Sema/Sema.h

9660 ↗

(On Diff #534778)

Maybe rename that BuildingDeductionGuidesTag

[ychen]

• address comments

[cor3ntin]

Please give @aaron.ballman a few days to get another look, but otherwise LGTM
Thanks a lot for working on this and your patience during the review.

[aaron.ballman]

LGTM aside from some minor issues, thank you!

clang/docs/ReleaseNotes.rst

98 ↗

(On Diff #535132)

Paren aggregate init is now supported, so this should be updated.

clang/lib/Sema/SemaInit.cpp

3271–3273

For local style consistency.

10688

clang/lib/Sema/SemaTemplate.cpp

2577–2580

I think this is a bit more clear as to what's happening.

[ychen]

• address comments

clang/docs/ReleaseNotes.rst

98 ↗

(On Diff #535132)

Thanks for catching this.

[ychen]

Thanks for the review!

[cor3ntin]

Thanks a lot for working on this!
Do you need Aaron or myself to commit this for you? If so, which name / mail do you want us to use?

[ychen]

In D139837#4460664, @cor3ntin wrote:

Thanks a lot for working on this!
Do you need Aaron or myself to commit this for you? If so, which name / mail do you want us to use?

That's okay. I'm doing the rebase and I could commit it later.

[ychen]

• rebase

[cor3ntin]

The changes to clang/www/cxx_status.html got lost during the rebase, can you fix that? Thanks!

[shafik]

So while working on D148474 I realized this PR introduced a new crash bug, see the following code: https://godbolt.org/z/h1EezGjbr

template<typename A3>
class B3 : A3 {
  template<bool = C3<B3>()>
  B3();
}; B3();

which is one of my test cases. I had though it was an interaction between my new code and this but then realized this exists w/o my new code. We can also see from the godbolt above that the crash in this code changed from an assertion to an unreachable. My PR will fix the assertion but I need a good solution to the unreachable bug. I tried changing the unreachable to a break but it reduces the quality of the diagnostic and I don't think it is the correct fix.

[shafik]

Note we have a second crash linked to this PR: https://github.com/llvm/llvm-project/issues/67173#issuecomment-1733647699

clang/include/clang/AST/DeclCXX.h

CXXDeductionGuideDecl(ASTContext &C, DeclContext *DC, SourceLocation StartLoc,
                         const DeclarationNameInfo &NameInfo, QualType T,
                         TypeSourceInfo *TInfo, SourceLocation EndLocation,
                         CXXConstructorDecl *Ctor)
       : FunctionDecl(CXXDeductionGuide, C, DC, StartLoc, NameInfo, T, TInfo,
                      SC_None, false, false, ConstexprSpecKind::Unspecified),
         Ctor(Ctor), ExplicitSpec(ES) {
     if (EndLocation.isValid())
       setRangeEnd(EndLocation);
     setDeductionCandidateKind(DeductionCandidateKind::Normal);
   }
 
   CXXConstructorDecl *Ctor;
   ExplicitSpecifier ExplicitSpec;
   void setExplicitSpecifier(ExplicitSpecifier ES) { ExplicitSpec = ES; }
 
 public:
   friend class ASTDeclReader;
TemplateDecl *getDeducedTemplate() const {
     return getDeclName().getCXXDeductionGuideTemplate();
   }
 
   /// Get the constructor from which this deduction guide was generated, if
   /// this is an implicit deduction guide.
   CXXConstructorDecl *getCorrespondingConstructor() const { return Ctor; }
 
   void setDeductionCandidateKind(DeductionCandidateKind K) {
     FunctionDeclBits.DeductionCandidateKind = K;
   }
 
   DeductionCandidateKind getDeductionCandidateKind() const {
     return FunctionDeclBits.DeductionCandidateKind;
   }
 
   bool isCopyDeductionCandidate() const {
     return getDeductionCandidateKind() == DeductionCandidateKind::Copy;
   }
 
-  bool isAggregateDeductionCandidate() const {
-    return getDeductionCandidateKind() == DeductionCandidateKind::Aggregate;
-  }
-
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == CXXDeductionGuide; }
 };
 
 /// \brief Represents the body of a requires-expression.
 ///
 /// This decl exists merely to serve as the DeclContext for the local
 /// parameters of the requires expression as well as other declarations inside

clang/lib/Sema/SemaInit.cpp

 //===--- SemaInit.cpp - Semantic Analysis for Initializers ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements semantic analysis for initializers.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 
 //===----------------------------------------------------------------------===//
 // Sema Initialization Checking
 //===----------------------------------------------------------------------===//
 
 /// Check whether T is compatible with a wide character type (wchar_t,
 /// char16_t or char32_t).

 class InitListChecker {
   Sema &SemaRef;
   bool hadError = false;
   bool VerifyOnly; // No diagnostics.
   bool TreatUnavailableAsInvalid; // Used only in VerifyOnly mode.
   bool InOverloadResolution;
   InitListExpr *FullyStructuredList = nullptr;
   NoInitExpr *DummyExpr = nullptr;
   SmallVector<QualType, 8> *AggrDeductionCandidateParamTypes = nullptr;
 
   NoInitExpr *getDummyInit() {
     if (!DummyExpr)
       DummyExpr = new (SemaRef.Context) NoInitExpr(SemaRef.Context.VoidTy);
     return DummyExpr;
   }
 
   void CheckImplicitInitList(const InitializedEntity &Entity,
void FillInEmptyInitializations(const InitializedEntity &Entity,
                                   bool FillWithNoInit = false);
   bool CheckFlexibleArrayInit(const InitializedEntity &Entity,
                               Expr *InitExpr, FieldDecl *Field,
                               bool TopLevelObject);
   void CheckEmptyInitializable(const InitializedEntity &Entity,
                                SourceLocation Loc);
 
 public:
   InitListChecker(Sema &S, const InitializedEntity &Entity, InitListExpr *IL,
                   QualType &T, bool VerifyOnly, bool TreatUnavailableAsInvalid,
                   bool InOverloadResolution = false,
                   SmallVector<QualType, 8> *AggrDeductionCandidateParamTypes = nullptr);
   InitListChecker(Sema &S, const InitializedEntity &Entity, InitListExpr *IL,
                   QualType &T,
                   SmallVector<QualType, 8> &AggrDeductionCandidateParamTypes)
       : InitListChecker(S, Entity, IL, T, true, false, false,
                         &AggrDeductionCandidateParamTypes) {
   };
 
   bool HadError() { return hadError; }
 
   // Retrieves the fully-structured initializer list used for
   // semantic analysis and code generation.
   InitListExpr *getFullyStructuredList() const { return FullyStructuredList; }
 };
 
 } // end anonymous namespace
 
for (const auto *Field : structDecl->fields())
     if (!Field->isUnnamedBitfield())
       ++InitializableMembers;
 
   if (structDecl->isUnion())
     return std::min(InitializableMembers, 1);
   return InitializableMembers - structDecl->hasFlexibleArrayMember();
 }
 
 RecordDecl *InitListChecker::getRecordDecl(QualType DeclType) {
   if (DeclType->isRecordType())
     return DeclType->castAs<RecordType>()->getDecl();
   if (auto *Inject = dyn_cast<InjectedClassNameType>(DeclType))
     return Inject->getDecl();
   return nullptr;
 }
 
 /// Determine whether Entity is an entity for which it is idiomatic to elide
 /// the braces in aggregate initialization.
 static bool isIdiomaticBraceElisionEntity(const InitializedEntity &Entity) {
   // Recursive initialization of the one and only field within an aggregate
   // class is considered idiomatic. This case arises in particular for
void InitListChecker::CheckImplicitInitList(const InitializedEntity &Entity,
   int maxElements = 0;
 
   if (T->isArrayType())
     maxElements = numArrayElements(T);
   else if (T->isRecordType())
     maxElements = numStructUnionElements(T);
   else if (T->isVectorType())
     maxElements = T->castAs<VectorType>()->getNumElements();
   else
     llvm_unreachable("CheckImplicitInitList(): Illegal type");
 
   if (maxElements == 0) {
     if (!VerifyOnly)
       SemaRef.Diag(ParentIList->getInit(Index)->getBeginLoc(),
                    diag::err_implicit_empty_initializer);
     ++Index;
     hadError = true;
     return;
if (SemaRef.getLangOpts().CPlusPlus || isa<InitListExpr>(expr)) {
     // we need initialization entity with a type of a vector (and not a vector
     // element!) initializing multiple vector elements.
     auto TmpEntity =
         (ElemType->isExtVectorType() && !Entity.getType()->isExtVectorType())
             ? InitializedEntity::InitializeTemporary(ElemType)
             : Entity;
 
     if (TmpEntity.getType()->isDependentType()) {
       // C++ [over.match.class.deduct]p1.5:
       //   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>(
               SemaRef.Context.getAsArrayType(ElemType))) {
         ++Index;
         AggrDeductionCandidateParamTypes->push_back(ElemType);
         return;
       }
for (RecordDecl::field_iterator FieldEnd = RD->field_end();
       }
     }
     return;
   }
 
   bool InitializedSomething = false;
 
   // If we have any base classes, they are initialized prior to the fields.
   for (auto I = Bases.begin(), E = Bases.end(); I != E; ++I) {
     auto &Base = *I;
     Expr *Init = Index < IList->getNumInits() ? IList->getInit(Index) : nullptr;
 
     // Designated inits always initialize fields, so if we see one, all
     // remaining base classes have no explicit initializer.
     if (Init && isa<DesignatedInitExpr>(Init))
       Init = nullptr;
 
     // C++ [over.match.class.deduct]p1.6:
     //   each non-trailing aggregate element that is a pack expansion is assumed
if (const ConstantArrayType *CAType = dyn_cast<ConstantArrayType>(AType)) {
       // initializer with many empty entries at the end.
       if (NumElements > ExpectedNumInits)
         NumElements = 0;
     }
   } else if (const VectorType *VType = CurrentObjectType->getAs<VectorType>()) {
     NumElements = VType->getNumElements();
   } else if (CurrentObjectType->isRecordType()) {
     NumElements = numStructUnionElements(CurrentObjectType);
   } else if (CurrentObjectType->isDependentType())
     NumElements = 1;
 
   Result->reserveInits(SemaRef.Context, NumElements);
 
   return Result;
 }
 
 /// Update the initializer at index @p StructuredIndex within the
 /// structured initializer list to the value @p expr.
 void InitListChecker::UpdateStructuredListElement(InitListExpr *StructuredList,
auto addDeductionCandidate = [&](FunctionTemplateDecl *TD,
 
     // C++ [over.match.list]p1.1: (first phase list initialization)
     //   Initially, the candidate functions are the initializer-list
     //   constructors of the class T
     if (OnlyListConstructors && !isInitListConstructor(GD))
       return;
 
     if (!AllowAggregateDeductionCandidate &&
-        GD->isAggregateDeductionCandidate())
+        GD->getDeductionCandidateKind() == DeductionCandidateKind::Aggregate)
       return;
 
     // C++ [over.match.list]p1.2: (second phase list initialization)
     //   the candidate functions are all the constructors of the class T
     // C++ [over.match.ctor]p1: (all other cases)
     //   the candidate functions are all the constructors of the class of
     //   the object being initialized
 
     // C++ [over.best.ics]p4:
     //   When [...] the constructor [...] is a candidate by
     //    - [over.match.copy] (in all cases)
     // FIXME: The "second phase of [over.match.list] case can also
     // theoretically happen here, but it's not clear whether we can
     // ever have a parameter of the right type.
     bool SuppressUserConversions = Kind.isCopyInit();
 
     if (TD) {
       SmallVector<Expr *, 8> TmpInits;
       for (Expr *E : Inits)
         if (auto *DI = dyn_cast<DesignatedInitExpr>(E))
           TmpInits.push_back(DI->getInit());
         else
           TmpInits.push_back(E);
-      AddTemplateOverloadCandidate(TD, FoundDecl, /*ExplicitArgs*/ nullptr,
+      AddTemplateOverloadCandidate(TD, FoundDecl, /*ExplicitArgs=*/nullptr,
                                    TmpInits, Candidates,
                                    SuppressUserConversions,
-                                   /*PartialOverloading*/ false, AllowExplicit,
+                                   /*PartialOverloading=*/false, AllowExplicit,
                                    ADLCallKind::NotADL, {}, true);
     } else {
       AddOverloadCandidate(GD, FoundDecl, Inits, Candidates,
                            SuppressUserConversions,
                            /*PartialOverloading*/ false, AllowExplicit);
     }
   };
 
   auto tryToResolveOverload =
       [&](bool OnlyListConstructors) -> OverloadingResult {
     Candidates.clear(OverloadCandidateSet::CSK_Normal);
     HasAnyDeductionGuide = false;
 
     for (auto I = Guides.begin(), E = Guides.end(); I != E; ++I) {
       NamedDecl *D = (*I)->getUnderlyingDecl();
       if (D->isInvalidDecl())
         continue;
 
       auto *TD = dyn_cast<FunctionTemplateDecl>(D);
       auto *GD = dyn_cast_or_null<CXXDeductionGuideDecl>(
           TD ? TD->getTemplatedDecl() : dyn_cast<FunctionDecl>(D));
       if (!GD)
         continue;
 
       if (!GD->isImplicit())
         HasAnyDeductionGuide = true;
 
       addDeductionCandidate(TD, GD, I.getPair(), OnlyListConstructors, false);
     }
 
     // C++ [over.match.class.deduct]p1.4:
     //   if C is defined and its definition satisfies the conditions for an
     //   aggregate class ([dcl.init.aggr]) with the assumption that any
     //   dependent base class has no virtual functions and no virtual base
     //   classes, and the initializer is a non-empty braced-init-list or
     //   parenthesized expression-list, and there are no deduction-guides for
     //   C, the set contains an additional function template, called the
     //   aggregate deduction candidate, defined as follows.
     if (!HasAnyDeductionGuide && ListInit && ListInit->getNumInits()) {
       // TODO: support parenthesized expression-list
       auto *RD = cast<CXXRecordDecl>(Template->getTemplatedDecl());
       if (RD->getDefinition() && RD->isAggregate()) {
         QualType Ty = Context.getRecordType(RD);
         SmallVector<QualType, 8> ElementTypes;
         InitListChecker CheckInitList(*this, Entity, ListInit, Ty,
                                       ElementTypes);
         if (!CheckInitList.HadError()) {
           // C++ [over.match.class.deduct]p1.8:
-          //   if e_i is of array type and x_i is a braced-init-list or
-          //   string-literal, T_i is an rvalue reference to the declared type
-          //   of e_i
+          //   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
+          //   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();
                i < e && !isa<PackExpansionType>(ElementTypes[i]); ++i)
             if (ElementTypes[i]->isArrayType()) {
               if (isa<InitListExpr>(ListInit->getInit(i)))
                 ElementTypes[i] =
                     Context.getRValueReferenceType(ElementTypes[i]);
-              else if (isa<StringLiteral>(ListInit->getInit(i)))
-                // This deviates from the wording which is incorrect.
-                // See https://github.com/cplusplus/draft/issues/3898.
+              else if (isa<StringLiteral>(
+                           ListInit->getInit(i)->IgnoreParenImpCasts()))
                 ElementTypes[i] =
                     Context.getLValueReferenceType(ElementTypes[i]);
             }
 
           llvm::FoldingSetNodeID ID;
           ID.AddPointer(Template);
           for (auto &T : ElementTypes)
             T.getCanonicalType().Profile(ID);
           unsigned Hash = ID.ComputeHash();
           if (AggregateDeductionCandidates.count(Hash) == 0) {
             if (FunctionTemplateDecl *TD =
                     DeclareImplicitDeductionGuideFromInitList(
                         Template, ElementTypes,
                         TSInfo->getTypeLoc().getEndLoc())) {
               auto *GD = cast<CXXDeductionGuideDecl>(TD->getTemplatedDecl());
               GD->setDeductionCandidateKind(DeductionCandidateKind::Aggregate);
               AggregateDeductionCandidates[Hash] = GD;
               addDeductionCandidate(TD, GD, DeclAccessPair::make(TD, AS_public),
                                     OnlyListConstructors, true);
             }
           } else {
             CXXDeductionGuideDecl *GD = AggregateDeductionCandidates[Hash];
             FunctionTemplateDecl *TD = GD->getDescribedFunctionTemplate();
-            assert(TD);
+            assert(TD && "aggregate deduction candidate is function template");
             addDeductionCandidate(TD, GD, DeclAccessPair::make(TD, AS_public),
                                   OnlyListConstructors, true);
           }
         }
       }
     }
 
     return Candidates.BestViableFunction(*this, Kind.getLocation(), Best);

clang/lib/Sema/SemaOverload.cpp

void Sema::AddMethodTemplateCandidate(
   //   functions. In such a case, the candidate functions generated from each
   //   function template are combined with the set of non-template candidate
   //   functions.
   TemplateDeductionInfo Info(CandidateSet.getLocation());
   FunctionDecl *Specialization = nullptr;
   ConversionSequenceList Conversions;
   if (TemplateDeductionResult Result = DeduceTemplateArguments(
           MethodTmpl, ExplicitTemplateArgs, Args, Specialization, Info,
           PartialOverloading, /*AggregateDeductionCandidate=*/false,
           [&](ArrayRef<QualType> ParamTypes) {
             return CheckNonDependentConversions(
                 MethodTmpl, ParamTypes, Args, CandidateSet, Conversions,
                 SuppressUserConversions, ActingContext, ObjectType,
                 ObjectClassification, PO);
           })) {
     OverloadCandidate &Candidate =
         CandidateSet.addCandidate(Conversions.size(), Conversions);
     Candidate.FoundDecl = FoundDecl;
void Sema::AddTemplateOverloadCandidate(
   }
 
   // Add the function template specialization produced by template argument
   // deduction as a candidate.
   assert(Specialization && "Missing function template specialization?");
   AddOverloadCandidate(
       Specialization, FoundDecl, Args, CandidateSet, SuppressUserConversions,
       PartialOverloading, AllowExplicit,
-      /*AllowExplicitConversions*/ false, IsADLCandidate, Conversions, PO,
+      /*AllowExplicitConversions=*/false, IsADLCandidate, Conversions, PO,
       Info.AggregateDeductionCandidateHasMismatchedArity);
 }
 
 /// Check that implicit conversion sequences can be formed for each argument
 /// whose corresponding parameter has a non-dependent type, per DR1391's
 /// [temp.deduct.call]p10.
 bool Sema::CheckNonDependentConversions(
     FunctionTemplateDecl *FunctionTemplate, ArrayRef<QualType> ParamTypes,
     ArrayRef<Expr *> Args, OverloadCandidateSet &CandidateSet,

clang/lib/Sema/SemaTemplate.cpp

FunctionTemplateDecl *buildDeductionGuide(
     return GuideTemplate;
   }
 };
 }
 
 FunctionTemplateDecl *Sema::DeclareImplicitDeductionGuideFromInitList(
     TemplateDecl *Template, MutableArrayRef<QualType> ParamTypes,
     SourceLocation Loc) {
   if (CXXRecordDecl *DefRecord =
           cast<CXXRecordDecl>(Template->getTemplatedDecl())->getDefinition()) {
     TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
     Template = DescribedTemplate ? DescribedTemplate : Template;
   }
 
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
     return nullptr;
 
   ConvertConstructorToDeductionGuideTransform Transform(
       *this, cast<ClassTemplateDecl>(Template));
   if (!isCompleteType(Loc, Transform.DeducedType))
     return nullptr;
 
   // In case we were expanding a pack when we attempted to declare deduction
   // guides, turn off pack expansion for everything we're about to do.
-  ArgumentPackSubstitutionIndexRAII SubstIndex(*this, -1);
+  ArgumentPackSubstitutionIndexRAII SubstIndex(*this,
+                                               /*NewSubstitutionIndex=*/-1);
   // Create a template instantiation record to track the "instantiation" of
   // constructors into deduction guides.
-  // FIXME: Add a kind for this to give more meaningful diagnostics. But can
-  // this substitution process actually fail?
+  // FIXME: Add a kind for this to give more meaningful diagnostics.
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
   if (BuildingDeductionGuides.isInvalid())
     return nullptr;
 
   return cast<FunctionTemplateDecl>(
       Transform.buildSimpleDeductionGuide(ParamTypes));
 }
 
for (auto *D : Existing)
     if (D->isImplicit())
       return;
 
   // In case we were expanding a pack when we attempted to declare deduction
   // guides, turn off pack expansion for everything we're about to do.
   ArgumentPackSubstitutionIndexRAII SubstIndex(*this, -1);
   // Create a template instantiation record to track the "instantiation" of
   // constructors into deduction guides.
-  // FIXME: Add a kind for this to give more meaningful diagnostics. But can
-  // this substitution process actually fail?
+  // FIXME: Add a kind for this to give more meaningful diagnostics.
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
   if (BuildingDeductionGuides.isInvalid())
     return;
 
   // Convert declared constructors into deduction guide templates.
   // FIXME: Skip constructors for which deduction must necessarily fail (those
   // for which some class template parameter without a default argument never
   // appears in a deduced context).

clang/lib/Sema/SemaTemplateDeduction.cpp

case TemplateArgument::Pack: {
     if (Y.getKind() != TemplateArgument::Pack ||
         (!AggregateCandidateDeduction && X.pack_size() != Y.pack_size()))
       return DeducedTemplateArgument();
 
     llvm::SmallVector<TemplateArgument, 8> NewPack;
     for (TemplateArgument::pack_iterator
              XA = X.pack_begin(),
              XAEnd = X.pack_end(), YA = Y.pack_begin(), YAEnd = Y.pack_end();
-         XA != XAEnd; ++XA) {
+         XA != XAEnd; ++XA, ++YA) {
       if (YA != YAEnd) {
         TemplateArgument Merged = checkDeducedTemplateArguments(
             Context, DeducedTemplateArgument(*XA, X.wasDeducedFromArrayBound()),
             DeducedTemplateArgument(*YA, Y.wasDeducedFromArrayBound()));
         if (Merged.isNull() && !(XA->isNull() && YA->isNull()))
           return DeducedTemplateArgument();
         NewPack.push_back(Merged);
-        ++YA;
       } else {
         NewPack.push_back(*XA);
       }
     }
 
     return DeducedTemplateArgument(
         TemplateArgument::CreatePackCopy(Context, NewPack),
         X.wasDeducedFromArrayBound() && Y.wasDeducedFromArrayBound());

 
 private:
   Sema &S;
   TemplateParameterList *TemplateParams;
   SmallVectorImpl<DeducedTemplateArgument> &Deduced;
   TemplateDeductionInfo &Info;
   unsigned PackElements = 0;
   bool IsPartiallyExpanded = false;
-  bool DeducePackIfNotAlreadyDeduced;
+  bool DeducePackIfNotAlreadyDeduced = false;
   /// The number of expansions, if we have a fully-expanded pack in this scope.
   std::optional<unsigned> FixedNumExpansions;
 
   SmallVector<DeducedPack, 2> Packs;
 };
 
 } // namespace