llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (Sirraide)

<details>
<summary>Changes</summary>

Sema would incorrectly skip diagnosing a malformed use of `= default` on an 
implcitly deleted move constructor while performing template instantiation, 
even if we were about to then generate a definition of said move constructor, 
which caused a crash.

This fixes that by always erroring in that case. However, there are some things 
to note here
1. This problem did not exist in earlier versions of Clang (e.g. the release 
branch of Clang 17 diagnoses this correctly with an overload resolution error). 
2. There are some other code paths that seem very similar to this one that that 
should probably also be investigated (specifically, other special member 
functions and potentially also defaulted comparison operators; I’ll take a look 
at adding some tests for those too if need be, but I wanted to document the 
current state of this here before that).
3. There may be a better place to put this check seeing as the functions that 
are changed by this do not seem to have been modified compared to the release 
branch of Clang 17, which also begs the question as to what change actually 
caused this to break.

This fixes #<!-- -->80869.

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


3 Files Affected:

- (modified) clang/include/clang/Sema/Sema.h (+2-1) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+12-6) 
- (modified) clang/test/SemaCXX/cxx0x-defaulted-functions.cpp (+20) 


``````````diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3c26003b5bda7f..7e36b3f1771a71 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7999,7 +7999,8 @@ class Sema final {
 
   bool CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
                                              CXXSpecialMember CSM,
-                                             SourceLocation DefaultLoc);
+                                             SourceLocation DefaultLoc,
+                                             bool ForDefinition = false);
   void CheckDelayedMemberExceptionSpecs();
 
   bool CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *MD,
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ab8a967b06a456..8f355e1e86519d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7676,7 +7676,8 @@ void Sema::CheckExplicitlyDefaultedFunction(Scope *S, 
FunctionDecl *FD) {
 
 bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
                                                  CXXSpecialMember CSM,
-                                                 SourceLocation DefaultLoc) {
+                                                 SourceLocation DefaultLoc,
+                                                 bool ForDefinition) {
   CXXRecordDecl *RD = MD->getParent();
 
   assert(MD->isExplicitlyDefaulted() && CSM != CXXInvalid &&
@@ -7894,13 +7895,18 @@ bool 
Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
   if (ShouldDeleteForTypeMismatch || ShouldDeleteSpecialMember(MD, CSM)) {
     if (First) {
       SetDeclDeleted(MD, MD->getLocation());
-      if (!inTemplateInstantiation() && !HadError) {
-        Diag(MD->getLocation(), diag::warn_defaulted_method_deleted) << CSM;
+      if ((ForDefinition || !inTemplateInstantiation()) && !HadError) {
+        // Always error if we're about to generate a definition.
+        HadError = ForDefinition;
+        Diag(MD->getLocation(), ForDefinition
+                                    ? diag::err_out_of_line_default_deletes
+                                    : diag::warn_defaulted_method_deleted)
+            << CSM;
         if (ShouldDeleteForTypeMismatch) {
           Diag(MD->getLocation(), diag::note_deleted_type_mismatch) << CSM;
         } else if (ShouldDeleteSpecialMember(MD, CSM, nullptr,
                                              /*Diagnose*/ true) &&
-                   DefaultLoc.isValid()) {
+                   DefaultLoc.isValid() && !ForDefinition) {
           Diag(DefaultLoc, diag::note_replace_equals_default_to_delete)
               << FixItHint::CreateReplacement(DefaultLoc, "delete");
         }
@@ -18285,8 +18291,8 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation 
DefaultLoc) {
   } else {
     auto *MD = cast<CXXMethodDecl>(FD);
 
-    if (CheckExplicitlyDefaultedSpecialMember(MD, DefKind.asSpecialMember(),
-                                              DefaultLoc))
+    if (CheckExplicitlyDefaultedSpecialMember(
+            MD, DefKind.asSpecialMember(), DefaultLoc, /*ForDefinition=*/true))
       MD->setInvalidDecl();
     else
       DefineDefaultedFunction(*this, MD, DefaultLoc);
diff --git a/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp 
b/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
index 0c3dd1ea7aa274..112bb4488de063 100644
--- a/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
+++ b/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
@@ -286,3 +286,23 @@ struct B {
   auto operator = (RM<B>) -> RV<B> = delete;
 };
 }
+
+namespace GH80869 {
+  struct F {F(F&&)=delete;}; // expected-note {{has been explicitly marked 
deleted here}}
+
+  template<int=0>
+  struct M {
+    F f; // expected-note {{field 'f' has a deleted move constructor}}
+    M();
+    M(const M&);
+    M(M&&);
+  };
+
+  template<int I>
+  M<I>::M(M&&)=default; // expected-error {{would delete it after its first 
declaration}}
+
+  M<> f() {
+    M<> m;
+    return m; // expected-note {{in instantiation of}}
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/80959
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to