Skip to content

Conversation

@mizvekov
Copy link
Contributor

Reverts #134038

This crashes clang as reported here: #134038 (comment)

@mizvekov mizvekov requested a review from zyn0217 April 15, 2025 22:32
@mizvekov mizvekov self-assigned this Apr 15, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 15, 2025

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

Reverts llvm/llvm-project#134038

This crashes clang as reported here: #134038 (comment)


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

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (-1)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+3)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+1-55)
  • (removed) clang/test/CodeGenCXX/local-class-instantiation.cpp (-64)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 166f26921cb71..ee69af5632f6e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -457,7 +457,6 @@ Bug Fixes to C++ Support
   by template argument deduction.
 - Clang is now better at instantiating the function definition after its use inside
   of a constexpr lambda. (#GH125747)
-- Fixed a local class member function instantiation bug inside dependent lambdas. (#GH59734), (#GH132208)
 - Clang no longer crashes when trying to unify the types of arrays with
   certain differences in qualifiers (this could happen during template argument
   deduction or when building a ternary operator). (#GH97005)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 0e81804f8c1e7..d2408a94ad0ab 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4126,6 +4126,9 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
       if (FunctionDecl *Pattern =
               Function->getInstantiatedFromMemberFunction()) {
 
+        if (Function->isIneligibleOrNotSelected())
+          continue;
+
         if (Function->getTrailingRequiresClause()) {
           ConstraintSatisfaction Satisfaction;
           if (CheckFunctionConstraints(Function, Satisfaction) ||
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index bf5a882ba4f12..5c80077f294c6 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5597,61 +5597,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
-  // Let the instantiation use the Pattern's DeclarationNameLoc, due to the
-  // following awkwardness:
-  //
-  //   1. There are out-of-tree users of getNameInfo().getSourceRange(), who
-  //   expect the source range of the instantiated declaration to be set to
-  //   point to the definition.
-  //
-  //   2. That getNameInfo().getSourceRange() might return the TypeLocInfo's
-  //   location it tracked.
-  //
-  //   3. Function might come from an (implicit) declaration, while the pattern
-  //   comes from a definition. In these cases, we need the PatternDecl's source
-  //   location.
-  //
-  // To that end, we need to more or less tweak the DeclarationNameLoc. However,
-  // we can't blindly copy the DeclarationNameLoc from the PatternDecl to the
-  // function, since it contains associated TypeLocs that should have already
-  // been transformed. So, we rebuild the TypeLoc for that purpose. Technically,
-  // we should create a new function declaration and assign everything we need,
-  // but InstantiateFunctionDefinition updates the declaration in place.
-  auto NameLocPointsToPattern = [&] {
-    DeclarationNameInfo PatternName = PatternDecl->getNameInfo();
-    DeclarationNameLoc PatternNameLoc = PatternName.getInfo();
-    switch (PatternName.getName().getNameKind()) {
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
-      break;
-    default:
-      // Cases where DeclarationNameLoc doesn't matter, as it merely contains a
-      // source range.
-      return PatternNameLoc;
-    }
-
-    TypeSourceInfo *TSI = Function->getNameInfo().getNamedTypeInfo();
-    // TSI might be null if the function is named by a constructor template id.
-    // E.g. S<T>() {} for class template S with a template parameter T.
-    if (!TSI) {
-      // We don't care about the DeclarationName of the instantiated function,
-      // but only the DeclarationNameLoc. So if the TypeLoc is absent, we do
-      // nothing.
-      return PatternNameLoc;
-    }
-
-    QualType InstT = TSI->getType();
-    // We want to use a TypeLoc that reflects the transformed type while
-    // preserving the source location from the pattern.
-    TypeLocBuilder TLB;
-    TLB.pushTrivial(
-        Context, InstT,
-        PatternNameLoc.getNamedTypeInfo()->getTypeLoc().getBeginLoc());
-    return DeclarationNameLoc::makeNamedTypeLoc(
-        TLB.getTypeSourceInfo(Context, InstT));
-  };
-  Function->setDeclarationNameLoc(NameLocPointsToPattern());
+  Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
 
   EnterExpressionEvaluationContext EvalContext(
       *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/test/CodeGenCXX/local-class-instantiation.cpp b/clang/test/CodeGenCXX/local-class-instantiation.cpp
deleted file mode 100644
index 34103a1ee55ef..0000000000000
--- a/clang/test/CodeGenCXX/local-class-instantiation.cpp
+++ /dev/null
@@ -1,64 +0,0 @@
-// RUN: %clang_cc1 -std=c++17 %s -emit-llvm -triple %itanium_abi_triple -o - | FileCheck %s
-
-namespace LambdaContainingLocalClasses {
-
-template <typename F>
-void GH59734() {
-  [&](auto param) {
-    struct Guard {
-      Guard() {
-        // Check that we're able to create DeclRefExpr to param at this point.
-        static_assert(__is_same(decltype(param), int), "");
-      }
-      ~Guard() {
-        static_assert(__is_same(decltype(param), int), "");
-      }
-      operator decltype(param)() {
-        return decltype(param)();
-      }
-    };
-    Guard guard;
-    param = guard;
-  }(42);
-}
-
-// Guard::Guard():
-// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardC2Ev
-// Guard::operator int():
-// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardcviEv
-// Guard::~Guard():
-// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardD2Ev
-
-struct S {};
-
-template <class T = void>
-auto GH132208 = [](auto param) {
-  struct OnScopeExit {
-    OnScopeExit() {
-      static_assert(__is_same(decltype(param), S), "");
-    }
-    ~OnScopeExit() {
-      static_assert(__is_same(decltype(param), S), "");
-    }
-    operator decltype(param)() {
-      return decltype(param)();
-    }
-  } pending;
-
-  param = pending;
-};
-
-void bar() {
-  GH59734<int>();
-
-  GH132208<void>(S{});
-}
-
-// OnScopeExit::OnScopeExit():
-// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitC2Ev
-// OnScopeExit::operator S():
-// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitcvS5_Ev
-// OnScopeExit::~OnScopeExit():
-// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitD2Ev
-
-} // namespace LambdaContainingLocalClasses

@mizvekov mizvekov merged commit 16980d5 into main Apr 15, 2025
10 of 13 checks passed
@mizvekov mizvekov deleted the revert-134038-perf/local-class-instantiation-bug branch April 15, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants