[Clang] C++ Templates: Refactor and fix TransformLambdaExpr's mishandling of TypeLocs

[yuxuanchen1997]

TreeTransform::TransformLambdaExpr has an outlier case in TransformLambdaExpr. The TransformFunctionProtoType call cannot use the getDerived() version because of some reasons I still don't fully understand, the TemplateInstantiator uses an extra LocalInstantiationScope for handling function prototypes. As a result, the current code construction cannot transform Lambda's types recursively, failing to handle cases like Attributed or MacroQualified Types.

This change cleans up that different handling by differentiating the lambda case in TemplateInstantiator and TreeTransform can just continue to use TransformType.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Yuxuan Chen (yuxuanchen1997)

Changes

TreeTransform::TransformLambdaExpr has an outlier case in TransformLambdaExpr. The TransformFunctionProtoType call cannot use the getDerived() version because of some reasons I still don't fully understand, the TemplateInstantiator uses an extra LocalInstantiationScope for handling function prototypes. As a result, the current code construction cannot transform Lambda's types recursively, failing to handle cases like Attributed or MacroQualified Types.

This change cleans up that different handling by differentiating the lambda case in TemplateInstantiator and TreeTransform can just continue to use TransformType.

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+14-5)
• (modified) clang/lib/Sema/TreeTransform.h (+27-44)
• (modified) clang/test/SemaCXX/template-instantiation.cpp (+15-1)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index fc80515b45e35b..2dc8047b468156 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1203,6 +1203,9 @@ namespace {
     // Whether to evaluate the C++20 constraints or simply substitute into them.
     bool EvaluateConstraints = true;
 
+    // Whether we are in the middle of transforming a lambda expression.
+    bool TransformingLambda = false;
+
   public:
     typedef TreeTransform<TemplateInstantiator> inherited;
 
@@ -1454,8 +1457,9 @@ namespace {
     ExprResult TransformLambdaExpr(LambdaExpr *E) {
       LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
       Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);
-
+      TransformingLambda = true;
       ExprResult Result = inherited::TransformLambdaExpr(E);
+      TransformingLambda = false;
       if (Result.isInvalid())
         return Result;
 
@@ -2180,10 +2184,15 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB,
                                  CXXRecordDecl *ThisContext,
                                  Qualifiers ThisTypeQuals,
                                  Fn TransformExceptionSpec) {
-  // We need a local instantiation scope for this function prototype.
-  LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
-  return inherited::TransformFunctionProtoType(
-      TLB, TL, ThisContext, ThisTypeQuals, TransformExceptionSpec);
+  if (TransformingLambda)
+    return inherited::TransformFunctionProtoType(
+        TLB, TL, ThisContext, ThisTypeQuals, TransformExceptionSpec);
+  else {
+    // We need a local instantiation scope for this function prototype.
+    LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
+    return inherited::TransformFunctionProtoType(
+        TLB, TL, ThisContext, ThisTypeQuals, TransformExceptionSpec);
+  }
 }
 
 ParmVarDecl *TemplateInstantiator::TransformFunctionTypeParam(
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 4463904b07211b..968d1971112b7c 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13629,59 +13629,42 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   // The transformation MUST be done in the CurrentInstantiationScope since
   // it introduces a mapping of the original to the newly created
   // transformed parameters.
-  TypeSourceInfo *NewCallOpTSI = nullptr;
-  {
-    auto OldCallOpTypeLoc =
-        E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
-
-    auto TransformFunctionProtoTypeLoc =
-        [this](TypeLocBuilder &TLB, FunctionProtoTypeLoc FPTL) -> QualType {
-      SmallVector<QualType, 4> ExceptionStorage;
-      TreeTransform *This = this; // Work around gcc.gnu.org/PR56135.
-      return this->TransformFunctionProtoType(
-          TLB, FPTL, nullptr, Qualifiers(),
-          [&](FunctionProtoType::ExceptionSpecInfo &ESI, bool &Changed) {
-            return This->TransformExceptionSpec(FPTL.getBeginLoc(), ESI,
-                                                ExceptionStorage, Changed);
-          });
-    };
+  auto OldCallOpTypeLoc =
+      E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
 
-    QualType NewCallOpType;
-    TypeLocBuilder NewCallOpTLBuilder;
+  TypeLocBuilder NewCallOpTLBuilder;
+  QualType NewCallOpType =
+      getDerived().TransformType(NewCallOpTLBuilder, OldCallOpTypeLoc);
 
-    if (auto ATL = OldCallOpTypeLoc.getAs<AttributedTypeLoc>()) {
-      NewCallOpType = this->TransformAttributedType(
-          NewCallOpTLBuilder, ATL,
-          [&](TypeLocBuilder &TLB, TypeLoc TL) -> QualType {
-            return TransformFunctionProtoTypeLoc(
-                TLB, TL.castAs<FunctionProtoTypeLoc>());
-          });
-    } else {
-      auto FPTL = OldCallOpTypeLoc.castAs<FunctionProtoTypeLoc>();
-      NewCallOpType = TransformFunctionProtoTypeLoc(NewCallOpTLBuilder, FPTL);
-    }
-
-    if (NewCallOpType.isNull())
-      return ExprError();
-    NewCallOpTSI =
-        NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
-  }
+  if (NewCallOpType.isNull())
+    return ExprError();
 
-  ArrayRef<ParmVarDecl *> Params;
-  if (auto ATL = NewCallOpTSI->getTypeLoc().getAs<AttributedTypeLoc>()) {
-    Params = ATL.getModifiedLoc().castAs<FunctionProtoTypeLoc>().getParams();
-  } else {
-    auto FPTL = NewCallOpTSI->getTypeLoc().castAs<FunctionProtoTypeLoc>();
-    Params = FPTL.getParams();
-  }
+  TypeSourceInfo *NewCallOpTSI =
+      NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
+
+  auto ExtractParams = [](TypeLoc TL) {
+    auto ExtractParamsImpl = [](auto Self,
+                                TypeLoc TL) -> ArrayRef<ParmVarDecl *> {
+      if (auto FPTL = TL.getAs<FunctionProtoTypeLoc>()) {
+        return FPTL.getParams();
+      } else if (auto ATL = TL.getAs<AttributedTypeLoc>()) {
+        return Self(Self, ATL.getModifiedLoc());
+      } else if (auto MQTL = TL.getAs<MacroQualifiedTypeLoc>()) {
+        return Self(Self, MQTL.getInnerLoc());
+      } else {
+        llvm_unreachable("Unhandled TypeLoc");
+      }
+    };
+    return ExtractParamsImpl(ExtractParamsImpl, TL);
+  };
 
   getSema().CompleteLambdaCallOperator(
       NewCallOperator, E->getCallOperator()->getLocation(),
       E->getCallOperator()->getInnerLocStart(),
       E->getCallOperator()->getTrailingRequiresClause(), NewCallOpTSI,
       E->getCallOperator()->getConstexprKind(),
-      E->getCallOperator()->getStorageClass(), Params,
-      E->hasExplicitResultType());
+      E->getCallOperator()->getStorageClass(),
+      ExtractParams(NewCallOpTSI->getTypeLoc()), E->hasExplicitResultType());
 
   getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
   getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
diff --git a/clang/test/SemaCXX/template-instantiation.cpp b/clang/test/SemaCXX/template-instantiation.cpp
index 8543af0d5428d0..42be88fef461ee 100644
--- a/clang/test/SemaCXX/template-instantiation.cpp
+++ b/clang/test/SemaCXX/template-instantiation.cpp
@@ -1,15 +1,29 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -ast-dump %s | FileCheck %s
 // expected-no-diagnostics
 
 namespace GH76521 {
 
+#define MYATTR __attribute__((preserve_most))
+
 template <typename T>
 void foo() {
+  // CHECK: FunctionDecl {{.*}} foo 'void ()'
   auto l = []() __attribute__((preserve_most)) {};
+  // CHECK: CXXMethodDecl {{.*}} operator() 'auto () __attribute__((preserve_most)) const' inline
+  auto l2 = [](T t) __attribute__((preserve_most)) -> T { return t; };
+  // CHECK: CXXMethodDecl {{.*}} operator() 'auto (int) const -> int __attribute__((preserve_most))':'auto (int) __attribute__((preserve_most)) const -> int' implicit_instantiation inline instantiated_fro
 }
 
+template <typename T>
 void bar() {
+  // CHECK: FunctionDecl {{.*}} bar 'void ()'
+  auto l = []() MYATTR {};
+  // CHECK: CXXMethodDecl {{.*}} operator() 'auto () __attribute__((preserve_most)) const' inline
+}
+
+int main() {
   foo<int>();
+  bar<int>();
 }
 
 }

[AaronBallman]

CC @cor3ntin and @erichkeane for feedback on the approach here

[Sirraide]

So as I’ve been looking into this as well, the changes that simplify how TransformLambdaExpr works make sense (though I think we can go even further here because even TransformFuncitonProtoType could probably just take a bool and not a callback, but I’ll take another look at that); I’m not convinced we should only create the instantiation scope if it’s a lambda, though, because that seems like it’s only fixing the symptoms of TransformAttributedType instantiating the same FunctionProtoType twice.

@yuxuanchen1997 If it’s alright with you, currently at least, it’s looking like it’d make more sense if I added (part of) the changes you’ve made here to my branch for this—because I don’t think one fix makes sense w/o the other, so keeping them in separate prs just adds unnecessary complexity to this...