Skip to content

[Clang][Sema] Don't set instantiated from function when rewriting operator<=> #91339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 8, 2024

Conversation

sdkrystian
Copy link
Member

The following snippet causes a crash:

template<typename T>
struct A 
{
    bool operator<=>(const A&) const requires true = default;
};

bool f(A<int> a) 
{
    return a != A<int>();
}

This occurs because during the rewrite from operator<=> to operator==, the "pattern" operator<=> function is set as the instantiated from function for the newly created operator== function. This is obviously incorrect, and this patch fixes it.

@sdkrystian sdkrystian requested review from shafik and erichkeane May 7, 2024 14:27
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 7, 2024
@llvmbot
Copy link
Member

llvmbot commented May 7, 2024

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clangd

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

The following snippet causes a crash:

template&lt;typename T&gt;
struct A 
{
    bool operator&lt;=&gt;(const A&amp;) const requires true = default;
};

bool f(A&lt;int&gt; a) 
{
    return a != A&lt;int&gt;();
}

This occurs because during the rewrite from operator&lt;=&gt; to operator==, the "pattern" operator&lt;=&gt; function is set as the instantiated from function for the newly created operator== function. This is obviously incorrect, and this patch fixes it.


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+13-11)
  • (modified) clang/test/CXX/class/class.compare/class.compare.default/p4.cpp (+11)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a85095e424b64..87c4489114c8f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -683,6 +683,9 @@ Bug Fixes to C++ Support
 - Fix an assertion failure when parsing an invalid members of an anonymous class. (#GH85447)
 - Fixed a misuse of ``UnresolvedLookupExpr`` for ill-formed templated expressions. Fixes (#GH48673), (#GH63243)
   and (#GH88832).
+- Fix a crash when an implicitly declared ``operator==`` function with a trailing requires-clause has its
+  constraints compared to that of another declaration.
+
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index d544cfac55ba3..fde2d920c785e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2269,16 +2269,18 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
                             TemplateArgumentList::CreateCopy(SemaRef.Context,
                                                              Innermost),
                                                 /*InsertPos=*/nullptr);
-  } else if (isFriend && D->isThisDeclarationADefinition()) {
-    // Do not connect the friend to the template unless it's actually a
-    // definition. We don't want non-template functions to be marked as being
-    // template instantiations.
-    Function->setInstantiationOfMemberFunction(D, TSK_ImplicitInstantiation);
-  } else if (!isFriend) {
-    // If this is not a function template, and this is not a friend (that is,
-    // this is a locally declared function), save the instantiation relationship
-    // for the purposes of constraint instantiation.
-    Function->setInstantiatedFromDecl(D);
+  } else if (FunctionRewriteKind == RewriteKind::None) {
+    if (isFriend && D->isThisDeclarationADefinition()) {
+      // Do not connect the friend to the template unless it's actually a
+      // definition. We don't want non-template functions to be marked as being
+      // template instantiations.
+      Function->setInstantiationOfMemberFunction(D, TSK_ImplicitInstantiation);
+    } else if (!isFriend) {
+      // If this is not a function template, and this is not a friend (that is,
+      // this is a locally declared function), save the instantiation
+      // relationship for the purposes of constraint instantiation.
+      Function->setInstantiatedFromDecl(D);
+    }
   }
 
   if (isFriend) {
@@ -2669,7 +2671,7 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
                          TemplateArgumentList::CreateCopy(SemaRef.Context,
                                                           Innermost),
                                               /*InsertPos=*/nullptr);
-  } else if (!isFriend) {
+  } else if (!isFriend && FunctionRewriteKind == RewriteKind::None) {
     // Record that this is an instantiation of a member function.
     Method->setInstantiationOfMemberFunction(D, TSK_ImplicitInstantiation);
   }
diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
index 534c3b34d8832..a9b2fd2b2230f 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
@@ -161,3 +161,14 @@ struct non_constexpr_type {
 
 my_struct<non_constexpr_type> obj; // cxx2a-note {{in instantiation of template class 'GH61238::my_struct<GH61238::non_constexpr_type>' requested here}}
 }
+
+namespace Constrained {
+  template<typename T>
+  struct A {
+    std::strong_ordering operator<=>(const A&) const requires true = default;
+  };
+
+  bool f(A<int> a) {
+    return a != A<int>();
+  }
+}

@sdkrystian sdkrystian force-pushed the crash-rewritten-oper branch from fc31665 to 0f29348 Compare May 8, 2024 00:08
@sdkrystian sdkrystian merged commit d4cf20c into llvm:main May 8, 2024
4 of 5 checks passed
@shafik
Copy link
Collaborator

shafik commented Oct 8, 2024

This regression: #104720 seems to be linked to this change.

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 clang-tools-extra clangd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants