diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -287,6 +287,8 @@ - Stop stripping CV qualifiers from the type of ``this`` when capturing it by value in a lambda. (`#50866 `_) +- Fix incorrect merging of member-like constrained friends across modules, as + described in C++ [temp.friend]/9. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -2653,11 +2653,6 @@ /// template. bool hasSameTemplateName(const TemplateName &X, const TemplateName &Y) const; - /// Determine whether two Friend functions are different because constraints - /// that refer to an enclosing template, according to [temp.friend] p9. - bool FriendsDifferByConstraints(const FunctionDecl *X, - const FunctionDecl *Y) const; - /// Determine whether the two declarations refer to the same entity. bool isSameEntity(const NamedDecl *X, const NamedDecl *Y) const; diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2537,6 +2537,10 @@ ->FunctionDeclBits.FriendConstraintRefersToEnclosingTemplate; } + /// Determine whether a function is a friend function that cannot be + /// redeclared outside of its class, per C++ [temp.friend]p9. + bool isMemberLikeConstrainedFriend() const; + /// Gets the kind of multiversioning attribute this declaration has. Note that /// this can return a value even if the function is not multiversion, such as /// the case of 'target'. diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -6593,34 +6593,6 @@ return true; } -bool ASTContext::FriendsDifferByConstraints(const FunctionDecl *X, - const FunctionDecl *Y) const { - // If these aren't friends, then they aren't friends that differ by - // constraints. - if (!X->getFriendObjectKind() || !Y->getFriendObjectKind()) - return false; - - // If the two functions share lexical declaration context, they are not in - // separate instantations, and thus in the same scope. - if (declaresSameEntity(cast(X->getLexicalDeclContext() - ->getRedeclContext()), - cast(Y->getLexicalDeclContext() - ->getRedeclContext()))) - return false; - - if (!X->getDescribedFunctionTemplate()) { - assert(!Y->getDescribedFunctionTemplate() && - "How would these be the same if they aren't both templates?"); - - // If these friends don't have constraints, they aren't constrained, and - // thus don't fall under temp.friend p9. Else the simple presence of a - // constraint makes them unique. - return X->getTrailingRequiresClause(); - } - - return X->FriendConstraintRefersToEnclosingTemplate(); -} - bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const { // Caution: this function is called by the AST reader during deserialization, // so it cannot rely on AST invariants being met. Non-trivial accessors @@ -6701,6 +6673,15 @@ return false; } + // Per C++20 [temp.over.link]/4, friends in different classes are sometimes + // not the same entity if they are constrained. + if ((FuncX->isMemberLikeConstrainedFriend() || + FuncY->isMemberLikeConstrainedFriend()) && + !FuncX->getLexicalDeclContext()->Equals( + FuncY->getLexicalDeclContext())) { + return false; + } + // The trailing require clause of instantiated function may change during // the semantic analysis. Trying to get the primary template function (if // exists) to compare the primary trailing require clause. @@ -6725,10 +6706,6 @@ PrimaryY->getTrailingRequiresClause())) return false; - // Constrained friends are different in certain cases, see: [temp.friend]p9. - if (FriendsDifferByConstraints(FuncX, FuncY)) - return false; - auto GetTypeAsWritten = [](const FunctionDecl *FD) { // Map to the first declaration that we've already merged into this one. // The TSI of redeclarations might not match (due to calling conventions diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3368,6 +3368,27 @@ return false; } +bool FunctionDecl::isMemberLikeConstrainedFriend() const { + // C++20 [temp.friend]p9: + // A non-template friend declaration with a requires-clause [or] + // a friend function template with a constraint that depends on a template + // parameter from an enclosing template [...] does not declare the same + // function or function template as a declaration in any other scope. + + // If this isn't a friend then it's not a member-like constrained friend. + if (!getFriendObjectKind()) { + return false; + } + + if (!getDescribedFunctionTemplate()) { + // If these friends don't have constraints, they aren't constrained, and + // thus don't fall under temp.friend p9. Else the simple presence of a + // constraint makes them unique. + return getTrailingRequiresClause(); + } + + return FriendConstraintRefersToEnclosingTemplate(); +} MultiVersionKind FunctionDecl::getMultiVersionKind() const { if (hasAttr()) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1161,15 +1161,6 @@ !shouldLinkPossiblyHiddenDecl(*I, New)) continue; - // C++20 [temp.friend] p9: A non-template friend declaration with a - // requires-clause shall be a definition. A friend function template - // with a constraint that depends on a template parameter from an - // enclosing template shall be a definition. Such a constrained friend - // function or function template declaration does not declare the same - // function or function template as a declaration in any other scope. - if (Context.FriendsDifferByConstraints(OldF, New)) - continue; - Match = *I; return Ovl_Match; } @@ -1286,6 +1277,12 @@ !FunctionParamTypesAreEqual(OldType, NewType))) return true; + // For member-like friends, the enclosing class is part of the signature. + if ((New->isMemberLikeConstrainedFriend() || + Old->isMemberLikeConstrainedFriend()) && + !New->getLexicalDeclContext()->Equals(Old->getLexicalDeclContext())) + return true; + if (NewTemplate) { // C++ [temp.over.link]p4: // The signature of a function template consists of its function diff --git a/clang/test/Modules/merge-constrained-friends.cpp b/clang/test/Modules/merge-constrained-friends.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Modules/merge-constrained-friends.cpp @@ -0,0 +1,65 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++2b %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++2b %t/Use.cpp -fprebuilt-module-path=%t -fsyntax-only -verify + +//--- A.cppm +module; +export module A; + +struct B {}; + +export template struct A : B { + friend constexpr const int *f(B) requires true { + static constexpr int result = N; + return &result; + } + + template + friend constexpr const int *g(B) requires (M >= 0) && (N >= 0) { + static constexpr int result = M * 10 + N; + return &result; + } +}; + +export inline A<1> a1; +export inline A<2> a2; +export inline A<3> a3; + +static_assert(f(a1) != f(a2) && f(a2) != f(a3)); +static_assert(g<1>(a1) != g<1>(a2) && g<1>(a2) != g<1>(a3)); + +static_assert(*f(a1) == 1); +static_assert(*f(a2) == 2); +static_assert(*f(a3) == 3); + +static_assert(*g<4>(a1) == 41); +static_assert(*g<5>(a2) == 52); +static_assert(*g<6>(a3) == 63); + +//--- Use.cpp +// expected-no-diagnostics +import A; + +// Try some instantiations we tried before and some we didn't. +static_assert(f(a1) != f(a2) && f(a2) != f(a3)); +static_assert(g<1>(a1) != g<1>(a2) && g<1>(a2) != g<1>(a3)); +static_assert(g<2>(a1) != g<2>(a2) && g<2>(a2) != g<2>(a3)); + +A<4> a4; +static_assert(f(a1) != f(a4) && f(a2) != f(a4) && f(a3) != f(a4)); +static_assert(g<3>(a1) != g<3>(a4)); + +static_assert(*f(a1) == 1); +static_assert(*f(a2) == 2); +static_assert(*f(a3) == 3); +static_assert(*f(a4) == 4); + +static_assert(*g<4>(a1) == 41); +static_assert(*g<5>(a2) == 52); +static_assert(*g<6>(a3) == 63); + +static_assert(*g<7>(a1) == 71); +static_assert(*g<8>(a4) == 84);