rsmith updated this revision to Diff 509818.
rsmith added a comment.
- Merge branch 'main' into merge-constrained-friends
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147068/new/
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
@@ -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
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ 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<TargetAttr>())
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ 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<Decl>(X->getLexicalDeclContext()
- ->getRedeclContext()),
- cast<Decl>(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
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ 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'.
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
@@ -287,6 +287,8 @@
- Stop stripping CV qualifiers from the type of ``this`` when capturing it by value in
a lambda.
(`#50866 <https://github.com/llvm/llvm-project/issues/50866>`_)
+- 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
