Addendum for https://reviews.llvm.org/D119544

[erichkeane]

Summary

Split out from https://reviews.llvm.org/D119544 for the purposes of simplifying review, this will be committed with that again when we re-merge.

clang/lib/Sema/SemaOverload.cpp

static bool checkArgPlaceholdersForOverload(Sema &S, MultiExprArg Args,
                                             UnbridgedCastsSet &unbridged) {
   for (unsigned i = 0, e = Args.size(); i != e; ++i)
     if (checkPlaceholderForOverload(S, Args[i], &unbridged))
       return true;
 
   return false;
 }
 
+static bool FriendMembersDifferByConstraints(Sema &S, DeclContext *CurContext,
+                                             FunctionDecl *Old,
+                                             FunctionDecl *New) {
+  // Only necessary/valid if we're instantiating a
+  // ClassTemplateSpecializationDecl.
+  if (CurContext->getDeclKind() != Decl::ClassTemplateSpecialization)
+    return false;
+
+  // Only when we're dealing with a friend function.
+  if (!Old->getFriendObjectKind() || !New->getFriendObjectKind())
+    return false;
+
+  // If the the two functions share lexical declaration context, they are not in
+  // separate instantations.
+  if (New->getLexicalDeclContext() == Old->getLexicalDeclContext())
+    return false;
+
+  auto *OldLexCtx =
+      dyn_cast<ClassTemplateSpecializationDecl>(Old->getLexicalDeclContext());
+  auto *NewLexCtx =
+      dyn_cast<ClassTemplateSpecializationDecl>(New->getLexicalDeclContext());
+
+  if (!OldLexCtx || !NewLexCtx)
+    return false;
+
+  auto InitAssocConstrs = [](FunctionDecl *FD,
+                             SmallVectorImpl<const Expr *> &AC) {
+    if (const auto *FT = FD->getDescribedFunctionTemplate())
+      FT->getAssociatedConstraints(AC);
+    else
+      FD->getAssociatedConstraints(AC);
+  };
+  SmallVector<const Expr *, 3> OldAC;
+  SmallVector<const Expr *, 3> NewAC;
+  InitAssocConstrs(Old, OldAC);
+  InitAssocConstrs(New, NewAC);
+
+  assert(OldAC.size() == NewAC.size() &&
+         "Should not have been identical unless constraints were the same?");
+
+  // At this point, we know the constraints lists should be the same.
+  auto *OldBegin = OldAC.begin();
+  auto *NewBegin = NewAC.begin();
+
+  auto SubstConstraint = [](Sema &S, ClassTemplateSpecializationDecl *LexCtx,
+                            const Expr *Constraint) {
+    Sema::ContextRAII SavedContext(S, LexCtx);
+    LocalInstantiationScope Scope(S);
+    EnterExpressionEvaluationContext EvalCtx(
+        S, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+    Sema::SFINAETrap Trap(S);
+    return S.SubstConstraintExpr(const_cast<Expr *>(Constraint),
+                                 S.getTemplateInstantiationArgs(LexCtx));
+  };
+
+  for (; OldBegin != OldAC.end(); ++OldBegin, ++NewBegin) {
+    ExprResult OldConverted = SubstConstraint(S, OldLexCtx, *OldBegin);
+    ExprResult NewConverted = SubstConstraint(S, NewLexCtx, *NewBegin);
+
+    // We can consider these different, since they depend on the instantiation,
+    // and cannot prove they are identical.
+    if (OldConverted.isInvalid() || NewConverted.isInvalid())
+      return true;
+
+    // profile & compare. If they are now different, they aren't equal.
+    llvm::FoldingSetNodeID NewID, OldID;
+    NewConverted.get()->Profile(NewID, S.getASTContext(), /*Canonical=*/true);
+    OldConverted.get()->Profile(OldID, S.getASTContext(), /*Canonical=*/true);
+    if (NewID != OldID)
+      return true;
+  }
+
+  // If we haven't found a differing constraint, these are the same.
+  return false;
+}
+
 /// Determine whether the given New declaration is an overload of the
 /// declarations in Old. This routine returns Ovl_Match or Ovl_NonFunction if
 /// New and Old cannot be overloaded, e.g., if New has the same signature as
 /// some function in Old (C++ 1.3.10) or if the Old declarations aren't
 /// functions (or function templates) at all. When it does return Ovl_Match or
 /// Ovl_NonFunction, MatchedDecl will point to the decl that New cannot be
 /// overloaded with. This decl may be a UsingShadowDecl on top of the underlying
 /// declaration.
if (FunctionDecl *OldF = OldD->getAsFunction()) {
           HideUsingShadowDecl(S, cast<UsingShadowDecl>(*I));
           continue;
         }
 
         if (!isa<FunctionTemplateDecl>(OldD) &&
             !shouldLinkPossiblyHiddenDecl(*I, New))
           continue;
 
+        // If this is a friend function currently being instantiated as a part
+        // of a ClassTemplateSpecializationDecl, it could have
+        // otherwise-identical-looking constraints that depend on the current
+        // instantiation. In this case, if the otherwise apparent 'match' and
+        // the new declaration differ by lexical declaration context (meaning
+        // different Class Template Specializations), AND one of the collected
+        // constraints seems to 'depend' on the current instantiation in some
+        // way, than these are not matches and are likely instead overloads.
+        // Note that an 'error' case might still be valid later (as it could be
+        // something that would be 'changed' at 'checking' time), but is proof
+        // we depend on the Class Template Specialization.
+        if (FriendMembersDifferByConstraints(*this, CurContext, OldF, New))
+          continue;
+
         Match = *I;
         return Ovl_Match;
       }
 
       // Builtins that have custom typechecking or have a reference should
       // not be overloadable or redeclarable.
       if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
         Match = *I;

clang/test/SemaTemplate/concepts.cpp

void test_dependent() {
   // expected-error@-1{{invalid reference to function 'foo'}}
   // expected-note@#CMV_FOO{{because 'IsInt<decltype(this->t)>' evaluated to false}}
   CMV2.foo2<float>();
   // expected-error@-1{{no matching member function for call to 'foo2'}}
   // expected-note@#CMV_FOO2{{constraints not satisfied}}
   // expected-note@#CMV_FOO2_REQ{{because 'IsInt<decltype(this->t)>' evaluated to false}}
 }
 } // namespace DeferredInstantiationInstScope
+
+namespace LibCXXOperatorRedef {
+template <typename T, typename U> struct is_same {
+  static constexpr bool value = false;
+};
+template <typename T> struct is_same<T, T> {
+  static constexpr bool value = false;
+};
+
+template <typename T, typename U>
+concept same_as = is_same<T, U>::value;
+
+// An issue found from libcxx when trying to commit the deferred concepts patch.
+// This caused an error of 'redefinition of funcN'.
+template <class _Tp> struct __range_adaptor_closure {
+  template <typename _View, typename _Closure>
+    requires same_as<_Tp, _Closure>
+  friend constexpr decltype(auto) R1func1(_View &&__view,
+                                        _Closure &&__closure){};
+  template <typename _View, typename _Closure>
+  friend constexpr decltype(auto) R1func2(_View &&__view,
+                                        _Closure &&__closure)
+    requires same_as<_Tp, _Closure>
+  {};
+  template <same_as<_Tp> _View, typename _Closure>
+  friend constexpr decltype(auto) R1func3(_View &&__view,
+                                        _Closure &&__closure)
+  {};
+};
+
+struct A : __range_adaptor_closure<A> {};
+struct B : __range_adaptor_closure<B> {};
+
+// These three fail because after the 1st pass of instantiation, they are still
+// identical.
+template <class _Tp> struct __range_adaptor_closure2 {
+  template <typename _View, typename _Closure>
+    requires same_as<_View, _Closure>
+  friend constexpr decltype(auto) R2func1(_View &&__view, // #FUNC1
+                                        _Closure &&__closure){};
+  template <typename _View, typename _Closure>
+  friend constexpr decltype(auto) R2func2(_View &&__view, // #FUNC2
+                                        _Closure &&__closure)
+    requires same_as<_View, _Closure>
+  {};
+  template <typename _View, same_as<_View> _Closure>
+  friend constexpr decltype(auto) R2func3(_View &&__view, // #FUNC3
+                                        _Closure &&__closure){};
+};
+
+struct A2 : __range_adaptor_closure2<A2> {};
+struct B2 : __range_adaptor_closure2<B2> {};
+// expected-error@#FUNC1{{redefinition of 'R2func1'}}
+// expected-note@-2{{in instantiation of template class}}
+// expected-note@#FUNC1{{previous definition is here}}
+// expected-error@#FUNC2{{redefinition of 'R2func2'}}
+// expected-note@#FUNC2{{previous definition is here}}
+// expected-error@#FUNC3{{redefinition of 'R2func3'}}
+// expected-note@#FUNC3{{previous definition is here}}
+
+// These three are fine, they all depend on the parent template parameter, so
+// are different despite ::type not being valid.
+template <class _Tp> struct __range_adaptor_closure3 {
+  template <typename _View, typename _Closure>
+    requires same_as<typename _Tp::type, _Closure>
+  friend constexpr decltype(auto) R3func1(_View &&__view,
+                                        _Closure &&__closure){};
+  template <typename _View, typename _Closure>
+  friend constexpr decltype(auto) R3func2(_View &&__view,
+                                        _Closure &&__closure)
+    requires same_as<typename _Tp::type, _Closure>
+  {};
+  template <same_as<typename _Tp::type> _View, typename _Closure>
+  friend constexpr decltype(auto) R3func3(_View &&__view,
+                                        _Closure &&__closure)
+  {};
+};
+
+struct A3 : __range_adaptor_closure3<A3> {};
+struct B3 : __range_adaptor_closure3<B3> {};
+
+template <class _Tp> struct __range_adaptor_closure4 {
+  template <typename _View, typename _Closure>
+    requires same_as<_Tp, _View>
+  // expected-note@+1{{previous definition is here}}
+  void foo1(_View &&, _Closure &&) {}
+  template <typename _View, typename _Closure>
+    requires same_as<_Tp, _View>
+  // expected-error@+1{{class member cannot be redeclared}}
+  void foo1(_View &&, _Closure &&) {}
+
+  template <typename _View, typename _Closure>
+  // expected-note@+1{{previous definition is here}}
+  void foo2(_View &&, _Closure &&)
+    requires same_as<_Tp, _View>
+  {}
+  template <typename _View, typename _Closure>
+  // expected-error@+1{{class member cannot be redeclared}}
+  void foo2(_View &&, _Closure &&)
+    requires same_as<_Tp, _View>
+  {}
+
+  template <same_as<_Tp> _View, typename _Closure>
+  // expected-note@+1{{previous definition is here}}
+  void foo3(_View &&, _Closure &&)
+  {}
+  template <same_as<_Tp> _View, typename _Closure>
+  // expected-error@+1{{class member cannot be redeclared}}
+  void foo3(_View &&, _Closure &&)
+  {}
+};
+
+// Requires instantiation to fail, so no errors here.
+template<class _Tp> struct __range_adaptor_closure5 {
+  template<same_as<_Tp> U>
+  friend void foo(){}
+  template<same_as<_Tp> U>
+  friend void foo(){}
+};
+
+template<class _Tp> struct __range_adaptor_closure6 {
+  template<same_as<_Tp> U>
+  friend void foo(){} // #RAC6FOO1
+  template<same_as<_Tp> U>
+  friend void foo(){} // #RAC6FOO2
+};
+struct A6 : __range_adaptor_closure6<A6> {};
+//expected-error@#RAC6FOO2{{redefinition of 'foo'}}
+//expected-note@-2{{in instantiation of template class}}
+//expected-note@#RAC6FOO1{{previous definition is here}}
+
+template<class T> struct S1 {
+  template<typename U>
+  friend void dupe(){} // #S1DUPE
+
+  template<typename U>
+  requires same_as<U, U>
+  friend void dupe2(){} // #S1DUPE2
+};
+template<class T> struct S2 {
+  template<typename U>
+  friend void dupe(){} // #S2DUPE
+
+  template<typename U>
+  requires same_as<U, U>
+  friend void dupe2(){} // #S2DUPE2
+};
+
+template<class T> struct S3 {
+  template<typename U>
+  requires same_as<T, U>
+  friend void dupe(){} // #S3DUPE
+};
+template<class T> struct S4 {
+  template<typename U>
+  requires same_as<T, U>
+  friend void dupe(){} // #S4DUPE
+};
+
+// Same as S3 and S4, but aren't instantiated with the same T.
+template<class T> struct S5 {
+  template<typename U>
+  requires same_as<T, U>
+  friend void not_dupe(){}
+};
+template<class T> struct S6 {
+  template<typename U>
+  requires same_as<T, U>
+  friend void not_dupe(){}
+};
+
+template<class T> struct S7 {
+  void not_dupe() requires same_as<T, T>{}
+};
+
+void useS() {
+  S1<int> s1;
+  S2<double> s2;
+  // expected-error@#S2DUPE{{redefinition}}
+  // expected-note@-2{{in instantiation of template class}}
+  // expected-note@#S1DUPE{{previous definition is here}}
+  // expected-error@#S2DUPE2{{redefinition}}
+  // expected-note@#S1DUPE2{{previous definition is here}}
+
+  S3<int> s3;
+  S4<int> s4;
+  // expected-error@#S4DUPE{{redefinition}}
+  // expected-note@-2{{in instantiation of template class}}
+  // expected-note@#S3DUPE{{previous definition is here}}
+
+  // OK, because only instantiated with different T.
+  S5<int> s5;
+  S6<double> s6;
+
+  S7<int> s7;
+}
+
+} // namespace LibCXXOperatorRedef