[C++17] Fix PR34970 - tweak overload resolution for class template deduction-guides in line with WG21's p0620r0.

In order to identify the copy deduction candidate, I considered two approaches:
  - attempt to determine whether an implicit guide is a copy deduction candidate by checking certain properties of its subsituted parameter during overload-resolution.
  - using one of the many bits (WillHaveBody) from FunctionDecl (that CXXDeductionGuideDecl inherits from) that are otherwise irrelevant for deduction guides

After some brittle gymnastics w the first strategy, I settled on the second, although to avoid confusion and to give that bit a better name, i turned it into a member of an anonymous union.

Given this identification 'bit', the tweak to overload resolution was a simple reordering of the deduction guide checks (in SemaOverload.cpp::isBetterOverloadCandidate), in-line with Jason Merrill's p0620r0 drafting which made it into the working paper.  Concordant with that, I made sure the copy deduction candidate is always added.


References:
See https://bugs.llvm.org/show_bug.cgi?id=34970 
See http://wg21.link/p0620r0

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@316292 91177308-0d34-0410-b5e6-96231b3b80d8

Author: faisalv

include/clang/AST/Decl.h

@@ -1678,10 +1678,18 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
   /// skipped.
   unsigned HasSkippedBody : 1;
 
-  /// Indicates if the function declaration will have a body, once we're done
-  /// parsing it.
-  unsigned WillHaveBody : 1;
-
+protected:
+  // Since a Deduction Guide [C++17] will never have a body, we can share the
+  // storage, and use a different name.
+  union {
+    /// Indicates if the function declaration will have a body, once we're done
+    /// parsing it.
+    unsigned WillHaveBody : 1;
+    /// Indicates that the Deduction Guide is the implicitly generated 'copy
+    /// deduction candidate' (is used during overload resolution).
+    unsigned IsCopyDeductionCandidate : 1;
+  };
+private:
   /// \brief End part of this FunctionDecl's source range.
   ///
   /// We could compute the full range in getSourceRange(). However, when we're

include/clang/AST/DeclCXX.h

@@ -1881,6 +1881,10 @@ class CXXDeductionGuideDecl : public FunctionDecl {
     if (EndLocation.isValid())
       setRangeEnd(EndLocation);
     IsExplicitSpecified = IsExplicit;
+
+    // IsCopyDeductionCandidate is a union variant member, so ensure it is the
+    // active member by storing to it.
+    IsCopyDeductionCandidate = false; 
   }
 
 public:
@@ -1903,6 +1907,12 @@ class CXXDeductionGuideDecl : public FunctionDecl {
     return getDeclName().getCXXDeductionGuideTemplate();
   }
 
+  void setIsCopyDeductionCandidate() {
+    IsCopyDeductionCandidate = true;
+  }
+
+  bool isCopyDeductionCandidate() const { return IsCopyDeductionCandidate; }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == CXXDeductionGuide; }

lib/Sema/SemaOverload.cpp

@@ -8965,12 +8965,6 @@ bool clang::isBetterOverloadCandidate(
     // C++14 [over.match.best]p1 section 2 bullet 3.
   }
 
-  //    -- F1 is generated from a deduction-guide and F2 is not
-  auto *Guide1 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand1.Function);
-  auto *Guide2 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand2.Function);
-  if (Guide1 && Guide2 && Guide1->isImplicit() != Guide2->isImplicit())
-    return Guide2->isImplicit();
-
   //    -- F1 is a non-template function and F2 is a function template
   //       specialization, or, if not that,
   bool Cand1IsSpecialization = Cand1.Function &&
@@ -9015,6 +9009,23 @@ bool clang::isBetterOverloadCandidate(
     // Inherited from sibling base classes: still ambiguous.
   }
 
+  // Check C++17 tie-breakers for deduction guides.
+  {
+    auto *Guide1 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand1.Function);
+    auto *Guide2 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand2.Function);
+    if (Guide1 && Guide2) {
+      //  -- F1 is generated from a deduction-guide and F2 is not
+      if (Guide1->isImplicit() != Guide2->isImplicit())
+        return Guide2->isImplicit();
+
+      //  -- F1 is the copy deduction candidate(16.3.1.8) and F2 is not
+      if (Guide1->isCopyDeductionCandidate())
+        return true;
+    }
+  }
+  
+
+
   // FIXME: Work around a defect in the C++17 guaranteed copy elision wording,
   // as combined with the resolution to CWG issue 243.
   //

lib/Sema/SemaTemplate.cpp

@@ -1837,7 +1837,6 @@ void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   // for which some class template parameter without a default argument never
   // appears in a deduced context).
   bool AddedAny = false;
-  bool AddedCopyOrMove = false;
   for (NamedDecl *D : LookupConstructors(Transform.Primary)) {
     D = D->getUnderlyingDecl();
     if (D->isInvalidDecl() || D->isImplicit())
@@ -1854,20 +1853,22 @@ void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
 
     Transform.transformConstructor(FTD, CD);
     AddedAny = true;
-
-    AddedCopyOrMove |= CD->isCopyOrMoveConstructor();
   }
 
-  // Synthesize an X() -> X<...> guide if there were no declared constructors.
-  // FIXME: The standard doesn't say (how) to do this.
+  // C++17 [over.match.class.deduct]
+  //    --  If C is not defined or does not declare any constructors, an
+  //    additional function template derived as above from a hypothetical
+  //    constructor C().
   if (!AddedAny)
     Transform.buildSimpleDeductionGuide(None);
 
-  // Synthesize an X(X<...>) -> X<...> guide if there was no declared constructor
-  // resembling a copy or move constructor.
-  // FIXME: The standard doesn't say (how) to do this.
-  if (!AddedCopyOrMove)
-    Transform.buildSimpleDeductionGuide(Transform.DeducedType);
+  //    -- An additional function template derived as above from a hypothetical
+  //    constructor C(C), called the copy deduction candidate.
+  cast<CXXDeductionGuideDecl>(
+      cast<FunctionTemplateDecl>(
+          Transform.buildSimpleDeductionGuide(Transform.DeducedType))
+          ->getTemplatedDecl())
+      ->setIsCopyDeductionCandidate();
 }
 
 /// \brief Diagnose the presence of a default template argument on a

lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -1651,11 +1651,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
   }
 
   FunctionDecl *Function;
-  if (auto *DGuide = dyn_cast<CXXDeductionGuideDecl>(D))
+  if (auto *DGuide = dyn_cast<CXXDeductionGuideDecl>(D)) {
     Function = CXXDeductionGuideDecl::Create(
-        SemaRef.Context, DC, D->getInnerLocStart(), DGuide->isExplicit(),
-        D->getNameInfo(), T, TInfo, D->getSourceRange().getEnd());
-  else {
+      SemaRef.Context, DC, D->getInnerLocStart(), DGuide->isExplicit(),
+      D->getNameInfo(), T, TInfo, D->getSourceRange().getEnd());
+    if (DGuide->isCopyDeductionCandidate())
+      cast<CXXDeductionGuideDecl>(Function)->setIsCopyDeductionCandidate();
+  } else {
     Function = FunctionDecl::Create(
         SemaRef.Context, DC, D->getInnerLocStart(), D->getNameInfo(), T, TInfo,
         D->getCanonicalDecl()->getStorageClass(), D->isInlineSpecified(),

lib/Serialization/ASTReaderDecl.cpp

@@ -1863,6 +1863,8 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) {
 
 void ASTDeclReader::VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) {
   VisitFunctionDecl(D);
+  if (Record.readInt())
+    D->setIsCopyDeductionCandidate();
 }
 
 void ASTDeclReader::VisitCXXMethodDecl(CXXMethodDecl *D) {

lib/Serialization/ASTWriterDecl.cpp

@@ -612,6 +612,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
 
 void ASTDeclWriter::VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) {
   VisitFunctionDecl(D);
+  Record.push_back(D->isCopyDeductionCandidate());
   Code = serialization::DECL_CXX_DEDUCTION_GUIDE;
 }
 

test/CXX/over/over.match/over.match.best/p1.cpp

@@ -25,13 +25,13 @@ namespace deduction_guide_example {
 
   // FIXME: The standard's example is wrong; we add a remove_ref<...> here to
   // fix it.
-  template<typename T, int N = remove_ref<T>::value> A(T&&, int*) -> A<T>;
+  template<typename T, int N = remove_ref<T>::value> A(T&&, int*) -> A<T**>;
   A a{1, 0};
   extern A<int> a;
-  A b{a, 0};
+  A b{a, 0}; // uses the implicit ctor that is more specialized
 
   A<int> *pa = &a;
-  A<A<int>&> *pb = &b;
+  A<int> *pb = &b;
 }
 
 // Partial ordering of function template specializations will be tested 

test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp

@@ -6,24 +6,26 @@ namespace Explicit {
   template<typename T> struct A {
     A(T);
     A(T*);
+	A(...);
   };
   template<typename T> A(T) -> A<T>;
-  template<typename T> explicit A(T*) -> A<T>; // expected-note {{explicit}}
+  template<typename T> explicit A(T*) -> A<T**>; // expected-note {{explicit}}
 
   int *p;
   A a(p);
   A b = p;
   A c{p};
   A d = {p}; // expected-error {{selected an explicit deduction guide}}
 
-  using X = A<int>;
-  using Y = A<int*>;
+  using X = A<int**>;
+  using Y = A<int>;  // uses the implicit guide, being more specialized than the eligible user-defined deduction guides.
 
   using X = decltype(a);
   using Y = decltype(b);
   using X = decltype(c);
 }
 
+
 namespace std {
   template<typename T> struct initializer_list {
     const T *ptr;
@@ -54,3 +56,32 @@ namespace p0702r1 {
   // between X<int> and X<float>.
   X xz = {z}; // expected-error {{no viable constructor or deduction guide}}
 }
+namespace pr34970 {
+//https://bugs.llvm.org/show_bug.cgi?id=34970
+
+template <typename X, typename Y> struct IsSame {
+    static constexpr bool value = false;
+};
+
+template <typename Z> struct IsSame<Z, Z> {
+    static constexpr bool value = true;
+};
+
+template <typename T> struct Optional {
+    template <typename U> Optional(U&&) { }
+};
+
+template <typename A> Optional(A) -> Optional<A>;
+
+int main() {
+    Optional opt(1729);
+    Optional dupe(opt);
+
+    static_assert(IsSame<decltype(opt), Optional<int>>::value);
+    static_assert(IsSame<decltype(dupe), Optional<int>>::value);
+    static_assert(!IsSame<decltype(dupe), Optional<Optional<int>>>::value);
+	return 0;
+}
+
+
+}