rsmith created this revision.
rsmith added a reviewer: aaron.ballman.
Herald added a project: All.
rsmith requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When a friend declaration has a requires-clause, and either it's a
non-template function or it's a function template whose requires-clause
depends on an enclosing template parameter, it is member-like for the
purpose of redeclaration checking. Specifically, the lexically enclosing
class becomes part of its signature, so it can only be redeclared by
another declaration within the same class.
Previously we supported this rule when parsing declarations, but not
when merging them across modules.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147068
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Decl.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Modules/merge-constrained-friends.cpp
Index: clang/test/Modules/merge-constrained-friends.cpp
===================================================================
--- /dev/null
+++ 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<int N> struct A : B {
+ friend constexpr const int *f(B) requires true {
+ static constexpr int result = N;
+ return &result;
+ }
+
+ template<int M>
+ 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);
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1155,15 +1155,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;
}
@@ -1280,6 +1271,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
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3348,6 +3348,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<TargetAttr>())
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6582,31 +6582,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 (X->getLexicalDeclContext() == Y->getLexicalDeclContext())
- 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 {
if (X == Y)
return true;
@@ -6683,6 +6658,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;
+ }
+
if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
FuncY->getTrailingRequiresClause()))
return false;
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2539,6 +2539,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'.
Index: clang/include/clang/AST/ASTContext.h
===================================================================
--- clang/include/clang/AST/ASTContext.h
+++ 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;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -179,6 +179,8 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field.
(`#60166 <https://github.com/llvm/llvm-project/issues/60166>`_)
+- Fix incorrect merging of member-like constrained friends across modules, as
+ described in C++ [temp.friend]/9.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
