Skip to content

Conversation

@a-tarasyuk
Copy link
Member

@a-tarasyuk a-tarasyuk commented Apr 22, 2025

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2025

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #135680


This PR addresses the issue where Clang failed to diagnose the redeclaration of defined friend functions as deleted/defaulted

struct S {
    friend int f() { return 0; }
    friend int f() = delete;
};

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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Parse/ParseCXXInlineMethods.cpp (+7)
  • (modified) clang/test/SemaCXX/cxx2c-delete-with-message.cpp (+9-13)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5ccd346a93b4f..4362c23ce4058 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -462,6 +462,8 @@ Bug Fixes in This Version
 - Fixed a crash when ``#embed`` appears as a part of a failed constant
   evaluation. The crashes were happening during diagnostics emission due to
   unimplemented statement printer. (#GH132641)
+- Clang now correctly diagnoses the deleted/defaulted redeclaration of defined
+  friend functions. (#GH135680)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index b1064eb02b907..ab19c4d6dd706 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -142,6 +142,13 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(
       SkipUntil(tok::semi);
     }
 
+    Decl *PrevDecl = FnD->getPreviousDecl();
+    if (PrevDecl && isa<FunctionDecl>(PrevDecl) &&
+        PrevDecl->getLexicalDeclContext() == FnD->getLexicalDeclContext()) {
+      Actions.CheckForFunctionRedefinition(FnD->getAsFunction(),
+                                           cast<FunctionDecl>(PrevDecl));
+    }
+
     return FnD;
   }
 
diff --git a/clang/test/SemaCXX/cxx2c-delete-with-message.cpp b/clang/test/SemaCXX/cxx2c-delete-with-message.cpp
index 5609da18c05aa..e4a68bdcf2a4d 100644
--- a/clang/test/SemaCXX/cxx2c-delete-with-message.cpp
+++ b/clang/test/SemaCXX/cxx2c-delete-with-message.cpp
@@ -274,26 +274,22 @@ void operators() {
 
 namespace gh135506 {
 struct a {
-  // FIXME: We currently don't diagnose these invalid redeclarations if the
-  // second declaration is defaulted or deleted. This probably needs to be
-  // handled in ParseCXXInlineMethodDef() after parsing the defaulted/deleted
-  // body.
-  friend consteval int f() { return 3; }
-  friend consteval int f() = delete("foo");
+  friend consteval int f() { return 3; }    // expected-note {{previous definition is here}}
+  friend consteval int f() = delete("foo"); // expected-error {{redefinition of 'f'}}
 
-  friend consteval int g() { return 3; }
-  friend consteval int g() = delete;
+  friend consteval int g() { return 3; } // expected-note {{previous definition is here}}
+  friend consteval int g() = delete;     // expected-error {{redefinition of 'g'}}
 
-  friend int h() { return 3; }
-  friend int h() = delete;
+  friend int h() { return 3; } // expected-note {{previous definition is here}}
+  friend int h() = delete;     // expected-error {{redefinition of 'h'}}
 
-  friend consteval int i() = delete; // expected-note {{previous definition is here}}
+  friend consteval int i() = delete;     // expected-note {{previous definition is here}}
   friend consteval int i() { return 3; } // expected-error {{redefinition of 'i'}}
 };
 
 struct b {
-  friend consteval bool operator==(b, b) { return true; } // expected-note {{previous declaration is here}}
-  friend consteval bool operator==(b, b) = default; // expected-error {{defaulting this equality comparison operator is not allowed because it was already declared outside the class}}
+  friend consteval bool operator==(b, b) { return true; } // expected-note {{previous definition is here}}
+  friend consteval bool operator==(b, b) = default;       // expected-error {{redefinition of 'operator=='}}
 };
 
 struct c {

@shafik shafik requested a review from Sirraide April 22, 2025 17:22
Copy link
Member

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more test cases:

int a();
struct A { friend int a() = delete; };

struct B { friend int b(); };
struct C { friend int b() = delete; };

@Sirraide
Copy link
Member

Sirraide commented Apr 22, 2025

Actually, I meant this of course:

void a() {}
struct A { friend void a() = delete; };
 
struct B { friend void b() {} };
struct C { friend void b() = delete; };

@a-tarasyuk a-tarasyuk requested review from Sirraide, shafik and zwuis April 23, 2025 08:31
Copy link
Contributor

@zwuis zwuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@a-tarasyuk
Copy link
Member Author

@Sirraide, thanks for the feedback. I've added requested tests

@a-tarasyuk a-tarasyuk closed this Apr 24, 2025
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.

6 participants