Skip to content
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

[Clang] Ensure default arguments in friend declarations are only allowed in defining declarations to prevent multiple reachable declarations #113777

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

a-tarasyuk
Copy link
Contributor

Fixes #113324

…ion pattern when adding instantiated params to the scope in friend functions with defaulted params
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 26, 2024

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #113324


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+4-4)
  • (modified) clang/test/CXX/temp/temp.res/p4.cpp (+7)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6a95337815174b..428ec8c87a432e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -561,6 +561,8 @@ Bug Fixes to C++ Support
   const-default-constructible even if a union member has a default member initializer.
   (#GH95854).
 - Fixed an assertion failure when evaluating an invalid expression in an array initializer (#GH112140)
+- Fixed an assertion failure caused by an invalid template instantiation pattern
+  for adding instantiated parameters to the scope in friend functions with defaulted parameters. (#GH113324).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 6a55861fe5af3b..cddfcc48312042 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3435,10 +3435,10 @@ bool Sema::SubstDefaultArgument(
       //   template<typename T> void f(T a, int = decltype(a)());
       //   void g() { f(0); }
       LIS = std::make_unique<LocalInstantiationScope>(*this);
-      FunctionDecl *PatternFD = FD->getTemplateInstantiationPattern(
-          /*ForDefinition*/ false);
-      if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
-        return true;
+      if (FunctionDecl *PatternFD =
+              FD->getTemplateInstantiationPattern(/*ForDefinition*/ false))
+        if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
+          return true;
     }
 
     runWithSufficientStackSpace(Loc, [&] {
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index f54d8649f5da88..62bd766e7e1140 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -185,3 +185,10 @@ template<typename T> struct S {
   friend void X::f(T::type);
 };
 }
+
+namespace GH113324 {
+template <typename = int> struct ct {
+  friend void f(ct, int = 0); // expected-error {{friend declaration specifying a default argument must be a definition}}
+};
+void test() { f(ct<>{}); }
+}

@a-tarasyuk a-tarasyuk changed the title [Clang] prevent assertion failure from an invalid template instantiation pattern when adding instantiated params to the scope in friend functions with defaulted params [Clang] Ensure default arguments in friend declarations are only allowed in defining declarations to prevent multiple reachable declarations Nov 6, 2024
Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

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

LGTM except for a couple of nits.

Thanks!


template<class T> using alias = int;
template<typename T> struct C {
friend void f3(C, int a = alias<T&>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
friend void f3(C, int a = alias<T&>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}
// FIXME: We miss diagnosing the default argument instantiation failure (forming reference to void)
friend void f3(C, int a = alias<T&>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}

Copy link
Contributor

Choose a reason for hiding this comment

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

(It's missing this change by the way)

clang/lib/Sema/SemaExpr.cpp Outdated Show resolved Hide resolved
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.

[clang] clang trunk crashes at clang::FunctionDecl::getNumParams()
4 participants