[SemaTemplateInstantiate] Handle lack of TypeSourceInfo on special member functions in templated lambdas

[bruno]

Summary

During template instantiation involving templated lambdas, clang could hit an assertion in TemplateDeclInstantiator::SubstFunctionType since the functions are not associated with any TypeSourceInfo:

assert(OldTInfo && "substituting function without type source info");

This path is triggered when using templated lambdas like the one added as a test to this patch. To fix this:

• Create TypeSourceInfos for special members and make sure the template instantiator can get through all patterns.
• Introduce a SpecialMemberTypeInfoRebuilder tree transform to rewrite such member function arguments. Without this, we get errors like:

error: only special member functions and comparison operators may be defaulted

since getDefaultedFunctionKind can't properly recognize these functions as special members as part of SetDeclDefaulted.

Fixes PR45828 and PR44848.

[bruno]

Ping!

[bruno]

Ping!

[bruno]

Ping.

[mdreseler]

I found this PR when investigating a compiler error that triggers the same exception and that can be tracked back to clang 3.5. First of all, thank you for looking into this.

I applied your path to 5b742a0c106fbed11779d6dd99854a6f97643524.

Here is your test case without explicit template lambdas. This, too, fails on trunk, but is fixed with your changes:

template <typename F>
void a(F f) {
  f.operator()(0);
}

template <typename F>
void b(F f) {
  a([=](auto i) {
    f.operator()(i);
  });
}

void c() {
  b([&](auto i) {
  });
}

This version, which is now C++14-conformant, can be compiled in c++20 and c++17 modes, but continues to fail with c++14 with the same assertion triggering.

Here is a much shorter example that shows the same behavior (broken everywhere on trunk, fixed by your changes for c++17 and 20, still broken for c++14):

void kill() {
  const auto lambda = [&](auto arg1) {};

  [&](auto arg2) {
    lambda.operator()(arg2);
  }(0);
}

Interestingly, in both cases the problem disappears if the lambda is called directly (i.e., without .operator()). In the case that brought me here, knowing this allowed me to replace lambda.operator()<true>(...) with lambda(std::true_type{}, ...) and circumventing the compiler bug.

Your patch also fixes https://bugs.llvm.org/show_bug.cgi?id=44848 for c++17 and upwards.

[bruno]

@mdreseler Thanks for trying it out with other testcases, lemme take one more look and make it work for your c++14 version as well. @rsmith does the approach in this patch sounds reasonable?

Interestingly, in both cases the problem disappears if the lambda is called directly (i.e., without .operator()). In the case that brought me here, knowing this allowed me to replace lambda.operator()<true>(...) with lambda(std::true_type{}, ...) and circumventing the compiler bug.

I didn't know the workaround, interesting!

[bruno]

Make this work for c++14 as well, update testcase to cover all possibilities mentioned. @mdreseler thanks for bringing this up.

[aaron.ballman]

clang/lib/Sema/SemaDeclCXX.cpp

13209–13210

clang/lib/Sema/SemaTemplateInstantiate.cpp

2820–2821

Under what circumstances could we get here where Pattern is not dependent?

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

2323–2325

4955–4958

What about a defaulted spaceship operator? Do we do the right thing there already?

4961–5015

[bruno]

Thanks for the review @aaron.ballman!

clang/lib/Sema/SemaTemplateInstantiate.cpp

2820–2821

Consider this code:

1 const auto lambda = [&](auto arg1) {};
2 [&](auto arg2) { lambda.operator()(arg2); }(0);

While parsing lambda.operator()(arg2) in line 1, clang calls to DiagnoseUseOfDecl, which will deduce / tree transform the CXXRecordDecl for the lambda in line 1. This record isn't dependent according to isDependentContext()'s requirements for CXXRecordDecl (and it doesn't seem to be missing anything?), since the implicit class doesn't have an associated ClassTemplateDecl nor the lambda and context are dependent. We do have a FunctionTemplateDecl as part of CXXRecordDecl (because of operator()), but that doesn't play a game here.

PerformDependentDiagnostics relies on isDependentContext(), which leads to an assertion after the TSI problem is fixed in other places in the patch, that's what initially prompted me to change this.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

4955–4958

That's a good question, I believe it's orthogonal here.

As you noticed, the current approach bails out for anything not a special member, and that's because it only tackles the testcases I was able to produce, I haven't been able to come up with spaceship related use case that triggers or needs this code path. So I rather not add any code for that right now. If you have one, lemme know! Anything in that direction would likely benefit from its own patch/revisions, so we can track possible regressions separately.

[bruno]

Update patch after @aaron.ballman review!

[aaron.ballman]

Nothing else of substance from me, but I don't feel confident in giving final sign off on this. LG, but please wait until one of the other reviewers accepts as well.

clang/lib/Sema/SemaTemplateInstantiate.cpp

2820–2821

Ah, thank you for the explanation!

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

2323–2325

No need for the parens around isa any longer (the || was removed).

4955–4958

I can't think of a specific test case I'm worried about, I was more just thinking "oh, here's another special member function you can write =default on.

[bruno]

Nothing else of substance from me, but I don't feel confident in giving final sign off on this. LG, but please wait until one of the other reviewers accepts as well.

No problemo, thanks for the review!

[bruno]

Update after last round of reviews.

@rsmith Is the approach reasonable or is there a better way to go about this?

[bruno]

Ping.

[bruno]

Ping

[bruno]

Ping

[bruno]

Ping

[aaron.ballman]

LGTM! I think we can address any concerns from @rsmith post commit at this point.

clang/lib/Sema/SemaDeclCXX.cpp

void Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
 
   LangAS AS = getDefaultCXXMethodAddrSpace();
   if (AS != LangAS::Default) {
     EPI.TypeQuals.addAddressSpace(AS);
   }
 
   auto QT = Context.getFunctionType(ResultTy, Args, EPI);
   SpecialMem->setType(QT);
+
+  // During template instantiation of implicit special member functions we need
+  // a reliable TypeSourceInfo for the function prototype in order to allow
+  // functions to be substituted.
+  if (inTemplateInstantiation() &&
+      cast<CXXRecordDecl>(SpecialMem->getParent())->isLambda()) {
+    TypeSourceInfo *TSI =
+        Context.getTrivialTypeSourceInfo(SpecialMem->getType());
+    SpecialMem->setTypeSourceInfo(TSI);
+  }
 }
 
 CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor(
                                                      CXXRecordDecl *ClassDecl) {
   // C++ [class.ctor]p5:
   //   A default constructor for a class X is a constructor of class X
   //   that can be called without an argument. If there is no
   //   user-declared constructor for class X, a default constructor is
if (getLangOpts().CUDA) {
     inferCUDATargetForImplicitSpecialMember(ClassDecl, CXXCopyConstructor,
                                             CopyConstructor,
                                             /* ConstRHS */ Const,
                                             /* Diagnose */ false);
   }
 
   setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
 
+  // During template instantiation of special member functions we need a
+  // reliable TypeSourceInfo for the parameter types in order to allow functions
+  // to be substituted.
+  TypeSourceInfo *TSI = nullptr;
+  if (inTemplateInstantiation() && ClassDecl->isLambda())
+    TSI = Context.getTrivialTypeSourceInfo(ArgType);
+
   // Add the parameter to the constructor.
-  ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyConstructor,
-                                               ClassLoc, ClassLoc,
-                                               /*IdentifierInfo=*/nullptr,
-                                               ArgType, /*TInfo=*/nullptr,
-                                               SC_None, nullptr);
+  ParmVarDecl *FromParam =
+      ParmVarDecl::Create(Context, CopyConstructor, ClassLoc, ClassLoc,
+                          /*IdentifierInfo=*/nullptr, ArgType,
+                          /*TInfo=*/TSI, SC_None, nullptr);
   CopyConstructor->setParams(FromParam);
 
   CopyConstructor->setTrivial(
       ClassDecl->needsOverloadResolutionForCopyConstructor()
           ? SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor)
           : ClassDecl->hasTrivialCopyConstructor());
 
   CopyConstructor->setTrivialForCall(

clang/lib/Sema/SemaTemplateInstantiate.cpp

Sema::InstantiateClass(SourceLocation PointOfInstantiation,
   if (TSK == TSK_ImplicitInstantiation) {
     Instantiation->setLocation(Pattern->getLocation());
     Instantiation->setLocStart(Pattern->getInnerLocStart());
     Instantiation->setBraceRange(Pattern->getBraceRange());
   }
 
   if (!Instantiation->isInvalidDecl()) {
     // Perform any dependent diagnostics from the pattern.
+    if (Pattern->isDependentContext())
       PerformDependentDiagnostics(Pattern, TemplateArgs);
 
     // Instantiate any out-of-line class template partial
     // specializations now.
     for (TemplateDeclInstantiator::delayed_partial_spec_iterator
               P = Instantiator.delayed_partial_spec_begin(),
            PEnd = Instantiator.delayed_partial_spec_end();
          P != PEnd; ++P) {
       if (!Instantiator.InstantiateClassTemplatePartialSpecialization(

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

 //===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
 //
 // 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 C++ template instantiation for declarations.
 //
 //===----------------------------------------------------------------------===/
 
+#include "TreeTransform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/AST/DependentDiagnostic.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
   else if (CXXRecordDecl *PatternPrev = getPreviousDeclForInstantiation(D)) {
     NamedDecl *Prev = SemaRef.FindInstantiatedDecl(D->getLocation(),
                                                    PatternPrev,
                                                    TemplateArgs);
     if (!Prev) return nullptr;
     PrevDecl = cast<CXXRecordDecl>(Prev);
   }
 
-  CXXRecordDecl *Record = CXXRecordDecl::Create(
-      SemaRef.Context, D->getTagKind(), Owner, D->getBeginLoc(),
-      D->getLocation(), D->getIdentifier(), PrevDecl);
+  CXXRecordDecl *Record = nullptr;
+  if (D->isLambda())
+    Record = CXXRecordDecl::CreateLambda(
+        SemaRef.Context, Owner, D->getLambdaTypeInfo(), D->getLocation(),
+        D->isDependentLambda(), D->isGenericLambda(),
+        D->getLambdaCaptureDefault());
+  else
+    Record = CXXRecordDecl::Create(SemaRef.Context, D->getTagKind(), Owner,
+                                   D->getBeginLoc(), D->getLocation(),
+                                   D->getIdentifier(), PrevDecl);
 
   // Substitute the nested name specifier, if any.
   if (SubstQualifier(D, Record))
     return nullptr;
 
   SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
                                                               StartingScope);
 
Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
   }
 
   ExplicitSpecifier InstantiatedExplicitSpecifier =
       instantiateExplicitSpecifier(SemaRef, TemplateArgs,
                                    ExplicitSpecifier::getFromDecl(D), D);
   if (InstantiatedExplicitSpecifier.isInvalid())
     return nullptr;
 
+  // Implicit destructors/constructors created for local classes in
+  // DeclareImplicit* (see SemaDeclCXX.cpp) might not have an associated TSI.
+  // Unfortunately there isn't enough context in those functions to
+  // conditionally populate the TSI without breaking non-template related use
+  // cases. Populate TSIs prior to calling SubstFunctionType to make sure we get
+  // a proper transformation.
+  if (cast<CXXRecordDecl>(D->getParent())->isLambda() &&
+      !D->getTypeSourceInfo() &&
+      isa<CXXConstructorDecl, CXXDestructorDecl>(D)) {
+    TypeSourceInfo *TSI =
+        SemaRef.Context.getTrivialTypeSourceInfo(D->getType());
+    D->setTypeSourceInfo(TSI);
+  }
+
   SmallVector<ParmVarDecl *, 4> Params;
   TypeSourceInfo *TInfo = SubstFunctionType(D, Params);
   if (!TInfo)
     return nullptr;
   QualType T = adjustFunctionTypeForInstantiation(SemaRef.Context, D, TInfo);
 
   if (TemplateParams && TemplateParams->size()) {
     auto *LastParam =
Method = CXXConstructorDecl::Create(
         TrailingRequiresClause);
     Method->setRangeEnd(Constructor->getEndLoc());
   } else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
     Method = CXXDestructorDecl::Create(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
         Destructor->isInlineSpecified(), false, Destructor->getConstexprKind(),
         TrailingRequiresClause);
     Method->setRangeEnd(Destructor->getEndLoc());
+    Method->setDeclName(SemaRef.Context.DeclarationNames.getCXXDestructorName(
+        SemaRef.Context.getCanonicalType(
+            SemaRef.Context.getTypeDeclType(Record))));
   } else if (CXXConversionDecl *Conversion = dyn_cast<CXXConversionDecl>(D)) {
     Method = CXXConversionDecl::Create(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
         Conversion->isInlineSpecified(), InstantiatedExplicitSpecifier,
         Conversion->getConstexprKind(), Conversion->getEndLoc(),
         TrailingRequiresClause);
   } else {
     StorageClass SC = D->isStatic() ? SC_Static : SC_None;
void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   // a function template specialization, since the template to be
   // instantiated already has references to locals properly substituted.
   bool MergeWithParentScope = false;
   if (CXXRecordDecl *Rec = dyn_cast<CXXRecordDecl>(Function->getDeclContext()))
     MergeWithParentScope =
         Rec->isLocalClass() && !Function->isFunctionTemplateSpecialization();
 
   LocalInstantiationScope Scope(*this, MergeWithParentScope);
+  auto RebuildTypeSourceInfoForDefaultSpecialMembers = [&]() {
+    // Special members might get their TypeSourceInfo set up w.r.t the
+    // PatternDecl context, in which case parameters could still be pointing
+    // back to the original class, make sure arguments are bound to the
+    // instantiated record instead.
+    assert(PatternDecl->isDefaulted() &&
+           "Special member needs to be defaulted");
+    auto PatternSM = getDefaultedFunctionKind(PatternDecl).asSpecialMember();
+    if (!(PatternSM == Sema::CXXCopyConstructor ||
+          PatternSM == Sema::CXXCopyAssignment ||
+          PatternSM == Sema::CXXMoveConstructor ||
+          PatternSM == Sema::CXXMoveAssignment))
+      return;
+
+    auto *NewRec = dyn_cast<CXXRecordDecl>(Function->getDeclContext());
+    const auto *PatternRec =
+        dyn_cast<CXXRecordDecl>(PatternDecl->getDeclContext());
+    if (!NewRec || !PatternRec)
+      return;
+    if (!PatternRec->isLambda())
+      return;
+
+    struct SpecialMemberTypeInfoRebuilder
+        : TreeTransform<SpecialMemberTypeInfoRebuilder> {
+      using Base = TreeTransform<SpecialMemberTypeInfoRebuilder>;
+      const CXXRecordDecl *OldDecl;
+      CXXRecordDecl *NewDecl;
+
+      SpecialMemberTypeInfoRebuilder(Sema &SemaRef, const CXXRecordDecl *O,
+                                     CXXRecordDecl *N)
+          : TreeTransform(SemaRef), OldDecl(O), NewDecl(N) {}
+
+      bool TransformExceptionSpec(SourceLocation Loc,
+                                  FunctionProtoType::ExceptionSpecInfo &ESI,
+                                  SmallVectorImpl<QualType> &Exceptions,
+                                  bool &Changed) {
+        return false;
+      }
 
-  if (PatternDecl->isDefaulted())
+      QualType TransformRecordType(TypeLocBuilder &TLB, RecordTypeLoc TL) {
+        const RecordType *T = TL.getTypePtr();
+        RecordDecl *Record = cast_or_null<RecordDecl>(
+            getDerived().TransformDecl(TL.getNameLoc(), T->getDecl()));
+        if (Record != OldDecl)
+          return Base::TransformRecordType(TLB, TL);
+
+        QualType Result = getDerived().RebuildRecordType(NewDecl);
+        if (Result.isNull())
+          return QualType();
+
+        RecordTypeLoc NewTL = TLB.push<RecordTypeLoc>(Result);
+        NewTL.setNameLoc(TL.getNameLoc());
+        return Result;
+      }
+    } IR{*this, PatternRec, NewRec};
+
+    TypeSourceInfo *NewSI = IR.TransformType(Function->getTypeSourceInfo());
+    Function->setType(NewSI->getType());
+    Function->setTypeSourceInfo(NewSI);
+
+    ParmVarDecl *Parm = Function->getParamDecl(0);
+    TypeSourceInfo *NewParmSI = IR.TransformType(Parm->getTypeSourceInfo());
+    Parm->setType(NewParmSI->getType());
+    Parm->setTypeSourceInfo(NewParmSI);
+  };
+
+  if (PatternDecl->isDefaulted()) {
+    RebuildTypeSourceInfoForDefaultSpecialMembers();
     SetDeclDefaulted(Function, PatternDecl->getLocation());
-  else {
+  } else {
     MultiLevelTemplateArgumentList TemplateArgs =
       getTemplateInstantiationArgs(Function, nullptr, false, PatternDecl);
 
     // Substitute into the qualifier; we can get a substitution failure here
     // through evil use of alias templates.
     // FIXME: Is CurContext correct for this? Should we go to the (instantiation
     // of the) lexical context of the pattern?
     SubstQualifier(*this, PatternDecl, Function, TemplateArgs);

clang/lib/Sema/TreeTransform.h

 QualType
 TreeTransform<Derived>::TransformFunctionProtoType(TypeLocBuilder &TLB,
                                                    FunctionProtoTypeLoc TL) {
   SmallVector<QualType, 4> ExceptionStorage;
   TreeTransform *This = this; // Work around gcc.gnu.org/PR56135.
   return getDerived().TransformFunctionProtoType(
       TLB, TL, nullptr, Qualifiers(),
       [&](FunctionProtoType::ExceptionSpecInfo &ESI, bool &Changed) {
-        return This->TransformExceptionSpec(TL.getBeginLoc(), ESI,
-                                            ExceptionStorage, Changed);
+        return This->getDerived().TransformExceptionSpec(
+            TL.getBeginLoc(), ESI, ExceptionStorage, Changed);
       });
 }
 
 template<typename Derived> template<typename Fn>
 QualType TreeTransform<Derived>::TransformFunctionProtoType(
     TypeLocBuilder &TLB, FunctionProtoTypeLoc TL, CXXRecordDecl *ThisContext,
     Qualifiers ThisTypeQuals, Fn TransformExceptionSpec) {
 

clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp

+// RUN: %clang_cc1 -std=c++20 -DEXPLICIT -verify %s
+// RUN: %clang_cc1 -std=c++17 -DEXPLICIT -verify -Wno-c++20-extensions %s
+// RUN: %clang_cc1 -std=c++14 -verify %s
+
+// expected-no-diagnostics
+
+#ifdef EXPLICIT
+
+template <typename F>
+void a(F &&f) {
+  f.template operator()<0>();
+}
+
+template <typename F>
+void b(F &&f) {
+  a([=]<int i>() {
+    f.template operator()<i>();
+  });
+}
+
+void c() {
+  b([&]<int i>() {
+  });
+}
+
+#endif
+
+template <typename F> void a1(F f) { f.operator()(0); }
+
+template <typename F> void b1(F f) {
+  a1([=](auto i) { f.operator()(i); });
+}
+
+void c1() {
+  b1([&](auto i) {});
+}
+
+void c2() {
+  const auto lambda = [&](auto arg1) {};
+  [&](auto arg2) { lambda.operator()(arg2); }(0);
+}