Skip to content

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 erichkeane and shafik 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>();
+  }
+}

@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