Factor out helper to determine whether a function is a "member-like
authorRichard Smith <richard@metafoo.co.uk>
Thu, 30 Mar 2023 23:07:55 +0000 (16:07 -0700)
committerRichard Smith <richard@metafoo.co.uk>
Fri, 31 Mar 2023 00:00:16 +0000 (17:00 -0700)
constrained friend".

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. In this change, we call such
functions "member-like constrained friends".

No functional change intended.

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 [new file with mode: 0644]

index 16503e6..e236255 100644 (file)
@@ -2653,11 +2653,6 @@ public:
   /// 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 05e309b..fde9573 100644 (file)
@@ -2537,6 +2537,10 @@ public:
         ->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 601c592..e7992e4 100644 (file)
@@ -6593,34 +6593,6 @@ static bool hasSameOverloadableAttrs(const FunctionDecl *A,
   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 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
         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 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
                               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 bc85372..de600e3 100644 (file)
@@ -3368,6 +3368,27 @@ bool FunctionDecl::isNoReturn() const {
   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 18b356c..f10d937 100644 (file)
@@ -1161,15 +1161,6 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
             !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 @@ bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
        !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 (file)
index 0000000..13147d5
--- /dev/null
@@ -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);