r295277 - Add missing "deduced A == A" check for function template partial ordering.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 15 19:49:44 PST 2017


Author: rsmith
Date: Wed Feb 15 21:49:44 2017
New Revision: 295277

URL: http://llvm.org/viewvc/llvm-project?rev=295277&view=rev
Log:
Add missing "deduced A == A" check for function template partial ordering.

This appears to be the only template argument deduction context where we were
missing this check. Surprisingly, other implementations also appear to miss
the check in this case; it may turn out that important code is relying on
the widespread non-conformance here, in which case we'll need to reconsider.

Modified:
    cfe/trunk/include/clang/Basic/Diagnostic.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
    cfe/trunk/test/CXX/drs/dr4xx.cpp
    cfe/trunk/test/SemaTemplate/deduction.cpp
    cfe/trunk/test/SemaTemplate/partial-order.cpp
    cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp
    cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Feb 15 21:49:44 2017
@@ -551,13 +551,15 @@ public:
   OverloadsShown getShowOverloads() const { return ShowOverloads; }
   
   /// \brief Pretend that the last diagnostic issued was ignored, so any
-  /// subsequent notes will be suppressed.
+  /// subsequent notes will be suppressed, or restore a prior ignoring
+  /// state after ignoring some diagnostics and their notes, possibly in
+  /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored() {
+  void setLastDiagnosticIgnored(bool Ignored = true) {
     if (LastDiagLevel == DiagnosticIDs::Fatal)
       FatalErrorOccurred = true;
-    LastDiagLevel = DiagnosticIDs::Ignored;
+    LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
   }
 
   /// \brief Determine whether the previous diagnostic was ignored. This can

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Feb 15 21:49:44 2017
@@ -7210,13 +7210,16 @@ public:
     unsigned PrevSFINAEErrors;
     bool PrevInNonInstantiationSFINAEContext;
     bool PrevAccessCheckingSFINAE;
+    bool PrevLastDiagnosticIgnored;
 
   public:
     explicit SFINAETrap(Sema &SemaRef, bool AccessCheckingSFINAE = false)
       : SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors),
         PrevInNonInstantiationSFINAEContext(
                                       SemaRef.InNonInstantiationSFINAEContext),
-        PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE)
+        PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE),
+        PrevLastDiagnosticIgnored(
+            SemaRef.getDiagnostics().isLastDiagnosticIgnored())
     {
       if (!SemaRef.isSFINAEContext())
         SemaRef.InNonInstantiationSFINAEContext = true;
@@ -7228,6 +7231,8 @@ public:
       SemaRef.InNonInstantiationSFINAEContext
         = PrevInNonInstantiationSFINAEContext;
       SemaRef.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
+      SemaRef.getDiagnostics().setLastDiagnosticIgnored(
+          PrevLastDiagnosticIgnored);
     }
 
     /// \brief Determine whether any SFINAE errors have been trapped.

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Wed Feb 15 21:49:44 2017
@@ -2242,7 +2242,7 @@ static Sema::TemplateDeductionResult Con
     SmallVectorImpl<DeducedTemplateArgument> &Deduced,
     TemplateDeductionInfo &Info, SmallVectorImpl<TemplateArgument> &Builder,
     LocalInstantiationScope *CurrentInstantiationScope = nullptr,
-    unsigned NumAlreadyConverted = 0, bool PartialOverloading = false) {
+    unsigned NumAlreadyConverted = 0, bool SkipNonDeduced = false) {
   TemplateParameterList *TemplateParams = Template->getTemplateParameters();
 
   for (unsigned I = 0, N = TemplateParams->size(); I != N; ++I) {
@@ -2330,11 +2330,14 @@ static Sema::TemplateDeductionResult Con
 
     // If there was no default argument, deduction is incomplete.
     if (DefArg.getArgument().isNull()) {
+      if (SkipNonDeduced) {
+        Builder.push_back(TemplateArgument());
+        continue;
+      }
+
       Info.Param = makeTemplateParameter(
           const_cast<NamedDecl *>(TemplateParams->getParam(I)));
       Info.reset(TemplateArgumentList::CreateCopy(S.Context, Builder));
-      if (PartialOverloading) break;
-
       return HasDefaultArg ? Sema::TDK_SubstitutionFailure
                            : Sema::TDK_Incomplete;
     }
@@ -3295,9 +3298,9 @@ static bool AdjustFunctionParmAndArgType
   return false;
 }
 
-static bool
-hasDeducibleTemplateParameters(Sema &S, FunctionTemplateDecl *FunctionTemplate,
-                               QualType T);
+static bool hasDeducibleTemplateParameters(Sema &S,
+                                           TemplateParameterList *Params,
+                                           QualType T);
 
 static Sema::TemplateDeductionResult DeduceTemplateArgumentsFromCallArgument(
     Sema &S, TemplateParameterList *TemplateParams, unsigned FirstInnerIndex,
@@ -3486,7 +3489,7 @@ Sema::TemplateDeductionResult Sema::Dedu
     //   Template argument deduction is done by comparing each function template
     //   parameter that contains template-parameters that participate in
     //   template argument deduction ...
-    if (!hasDeducibleTemplateParameters(*this, FunctionTemplate, ParamType))
+    if (!hasDeducibleTemplateParameters(*this, TemplateParams, ParamType))
       return Sema::TDK_Success;
 
     //   ... with the type of the corresponding argument
@@ -4365,6 +4368,7 @@ static bool isAtLeastAsSpecializedAs(Sem
   //   The types used to determine the ordering depend on the context in which
   //   the partial ordering is done:
   TemplateDeductionInfo Info(Loc);
+  SmallVector<QualType, 4> Args1;
   SmallVector<QualType, 4> Args2;
   switch (TPOC) {
   case TPOC_Call: {
@@ -4388,8 +4392,6 @@ static bool isAtLeastAsSpecializedAs(Sem
     //
     // C++98/03 doesn't have this provision but we've extended DR532 to cover
     // it as wording was broken prior to it.
-    SmallVector<QualType, 4> Args1;
-
     unsigned NumComparedArguments = NumCallArguments1;
 
     if (!Method2 && Method1 && !Method1->isStatic()) {
@@ -4413,35 +4415,37 @@ static bool isAtLeastAsSpecializedAs(Sem
       Args1.resize(NumComparedArguments);
     if (Args2.size() > NumComparedArguments)
       Args2.resize(NumComparedArguments);
-    if (DeduceTemplateArguments(S, TemplateParams, Args2.data(), Args2.size(),
-                                Args1.data(), Args1.size(), Info, Deduced,
-                                TDF_None, /*PartialOrdering=*/true))
-      return false;
-
     break;
   }
 
   case TPOC_Conversion:
     //   - In the context of a call to a conversion operator, the return types
     //     of the conversion function templates are used.
-    if (DeduceTemplateArgumentsByTypeMatch(
-            S, TemplateParams, Proto2->getReturnType(), Proto1->getReturnType(),
-            Info, Deduced, TDF_None,
-            /*PartialOrdering=*/true))
-      return false;
+    Args1.push_back(Proto1->getReturnType());
+    Args2.push_back(Proto2->getReturnType());
     break;
 
   case TPOC_Other:
     //   - In other contexts (14.6.6.2) the function template's function type
     //     is used.
-    if (DeduceTemplateArgumentsByTypeMatch(S, TemplateParams,
-                                           FD2->getType(), FD1->getType(),
-                                           Info, Deduced, TDF_None,
-                                           /*PartialOrdering=*/true))
-      return false;
+    Args1.push_back(FD1->getType());
+    Args2.push_back(FD2->getType());
     break;
   }
 
+  // FIXME: C++1z [temp.deduct.partial]p4:
+  //   If a particular P contains no template-parameters that participate in
+  //   template argument deduction, that P is not used to determine the
+  //   ordering.
+  // We do not implement this because it has highly undesirable consequences;
+  // for instance, it means a non-dependent template is never more specialized
+  // than any other.
+
+  if (DeduceTemplateArguments(S, TemplateParams, Args2.data(), Args2.size(),
+                              Args1.data(), Args1.size(), Info, Deduced,
+                              TDF_None, /*PartialOrdering=*/true))
+    return false;
+
   // C++0x [temp.deduct.partial]p11:
   //   In most cases, all template parameters must have values in order for
   //   deduction to succeed, but for partial ordering purposes a template
@@ -4453,43 +4457,75 @@ static bool isAtLeastAsSpecializedAs(Sem
     if (Deduced[ArgIdx].isNull())
       break;
 
-  // FIXME: We fail to implement [temp.deduct.type]p1 along this path. We need
-  // to substitute the deduced arguments back into the template and check that
-  // we get the right type.
-
-  if (ArgIdx == NumArgs) {
-    // All template arguments were deduced. FT1 is at least as specialized
-    // as FT2.
-    return true;
+  if (ArgIdx != NumArgs) {
+    // At least one template argument was not deduced. Check whether we deduced
+    // everything that was used in the types used for ordering.
+
+    // Figure out which template parameters were used.
+    llvm::SmallBitVector UsedParameters(TemplateParams->size());
+    for (QualType T : Args2)
+      ::MarkUsedTemplateParameters(S.Context, T, false,
+                                   TemplateParams->getDepth(), UsedParameters);
+
+    for (; ArgIdx != NumArgs; ++ArgIdx)
+      // If this argument had no value deduced but was used in one of the types
+      // used for partial ordering, then deduction fails.
+      if (Deduced[ArgIdx].isNull() && UsedParameters[ArgIdx])
+        return false;
   }
 
-  // Figure out which template parameters were used.
-  llvm::SmallBitVector UsedParameters(TemplateParams->size());
-  switch (TPOC) {
-  case TPOC_Call:
-    for (unsigned I = 0, N = Args2.size(); I != N; ++I)
-      ::MarkUsedTemplateParameters(S.Context, Args2[I], false,
-                                   TemplateParams->getDepth(),
-                                   UsedParameters);
-    break;
+  EnterExpressionEvaluationContext Unevaluated(S, Sema::Unevaluated);
+  Sema::SFINAETrap Trap(S);
 
-  case TPOC_Conversion:
-    ::MarkUsedTemplateParameters(S.Context, Proto2->getReturnType(), false,
-                                 TemplateParams->getDepth(), UsedParameters);
-    break;
+  SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
+  Sema::InstantiatingTemplate Inst(
+      S, Info.getLocation(), FT2, DeducedArgs,
+      Sema::ActiveTemplateInstantiation::DeducedTemplateArgumentSubstitution,
+      Info);
+  if (Inst.isInvalid())
+    return false;
 
-  case TPOC_Other:
-    ::MarkUsedTemplateParameters(S.Context, FD2->getType(), false,
-                                 TemplateParams->getDepth(),
-                                 UsedParameters);
-    break;
-  }
+  SmallVector<TemplateArgument, 4> Builder;
+  if (ConvertDeducedTemplateArguments(S, FT2, true, Deduced, Info, Builder,
+                                      nullptr, 0, /*SkipNonDeduced*/true))
+    return false;
 
-  for (; ArgIdx != NumArgs; ++ArgIdx)
-    // If this argument had no value deduced but was used in one of the types
-    // used for partial ordering, then deduction fails.
-    if (Deduced[ArgIdx].isNull() && UsedParameters[ArgIdx])
+  // C++1z [temp.deduct.type]p1:
+  //   an attempt is made to find template argument values (a type for a type
+  //   parameter, a value for a non-type parameter, or a template for a
+  //   template parameter) that will make P, after substitution of the deduced
+  //   values (call it the deduced A), compatible with A.
+  TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack, Builder);
+  MultiLevelTemplateArgumentList Args(TemplateArgs);
+  auto *TrailingPack =
+      Args2.empty() ? nullptr : dyn_cast<PackExpansionType>(Args2.back());
+  unsigned PackIndex = Args2.size() - 1;
+  for (unsigned I = 0, N = Args1.size(); I != N; ++I) {
+    // Per C++ [temp.deduct.partial]p8, we're supposed to have formed separate
+    // P/A pairs for each parameter of template 1 for a trailing pack in
+    // template 2. Reconstruct those now if necessary.
+    QualType DeducedA;
+    if (TrailingPack && I >= PackIndex) {
+      Sema::ArgumentPackSubstitutionIndexRAII Index(S, I - PackIndex);
+      DeducedA = S.SubstType(TrailingPack->getPattern(), Args,
+                             Info.getLocation(), FT2->getDeclName());
+    } else {
+      DeducedA = S.SubstType(Args2[I], Args, Info.getLocation(),
+                             FT2->getDeclName());
+    }
+    if (DeducedA.isNull())
+      return false;
+
+    QualType A = Args1[I];
+    if (auto *AsPack = dyn_cast<PackExpansionType>(A))
+      A = AsPack->getPattern();
+
+    // Per [temp.deduct.partial]p5-7, we strip off top-level references and
+    // cv-qualifications before this check.
+    if (!S.Context.hasSameUnqualifiedType(DeducedA.getNonReferenceType(),
+                                          A.getNonReferenceType()))
       return false;
+  }
 
   return true;
 }
@@ -5303,17 +5339,13 @@ void Sema::MarkDeducedTemplateParameters
                                  true, TemplateParams->getDepth(), Deduced);
 }
 
-bool hasDeducibleTemplateParameters(Sema &S,
-                                    FunctionTemplateDecl *FunctionTemplate,
+bool hasDeducibleTemplateParameters(Sema &S, TemplateParameterList *Params,
                                     QualType T) {
   if (!T->isDependentType())
     return false;
 
-  TemplateParameterList *TemplateParams
-    = FunctionTemplate->getTemplateParameters();
-  llvm::SmallBitVector Deduced(TemplateParams->size());
-  ::MarkUsedTemplateParameters(S.Context, T, true, TemplateParams->getDepth(),
-                               Deduced);
+  llvm::SmallBitVector Deduced(Params->size());
+  ::MarkUsedTemplateParameters(S.Context, T, true, Params->getDepth(), Deduced);
 
   return Deduced.any();
 }

Modified: cfe/trunk/test/CXX/drs/dr4xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr4xx.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr4xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr4xx.cpp Wed Feb 15 21:49:44 2017
@@ -54,7 +54,7 @@ namespace dr401 { // dr401: yes
   void g(B b) { f(b); }
 #if __cplusplus < 201103L
   // expected-error at -3 0-1{{extension}} expected-error at -3 {{protected}} expected-note at -3 {{instantiation}}
-  // expected-note at -3 {{substituting}}
+  // expected-note at -3 {{substituting}} expected-note at -33 {{declared protected here}}
 #else
   // expected-error at -5 {{no matching}} expected-note at -6 {{protected}}
 #endif

Modified: cfe/trunk/test/SemaTemplate/deduction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/deduction.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/deduction.cpp (original)
+++ cfe/trunk/test/SemaTemplate/deduction.cpp Wed Feb 15 21:49:44 2017
@@ -127,12 +127,12 @@ namespace test1 {
 namespace test2 {
   template<typename T> struct Const { typedef void const type; };
 
-  template<typename T> void f(T, typename Const<T>::type*);
-  template<typename T> void f(T, void const *);
+  template<typename T> void f(T, typename Const<T>::type*); // expected-note {{candidate}}
+  template<typename T> void f(T, void const *); // expected-note {{candidate}}
 
   void test() {
     void *p = 0;
-    f(0, p);
+    f(0, p); // expected-error {{ambiguous}}
   }
 }
 

Modified: cfe/trunk/test/SemaTemplate/partial-order.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/partial-order.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/partial-order.cpp (original)
+++ cfe/trunk/test/SemaTemplate/partial-order.cpp Wed Feb 15 21:49:44 2017
@@ -1,7 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z %s -verify
 
-// expected-no-diagnostics
-
 namespace hana_enable_if_idiom {
   template<bool> struct A {};
   template<typename, typename = A<true>> struct B;
@@ -12,3 +10,21 @@ namespace hana_enable_if_idiom {
   };
   B<C> b;
 }
+
+// Ensure that we implement the check that deduced A == A during function
+// template partial ordering.
+namespace check_substituted_type_matches {
+  struct X { typedef int type; };
+
+  // A specific but dependent type is neither better nor worse than a
+  // specific and non-dependent type.
+  template<typename T> void f(T, typename T::type); // expected-note {{candidate}}
+  template<typename T> void f(T, int); // expected-note {{candidate}}
+  void test_f() { f(X{}, 0); } // expected-error {{ambiguous}}
+
+  // A specific but dependent type is more specialized than a
+  // deducible type.
+  template<typename T> void g(T, typename T::type);
+  template<typename T, typename U> void g(T, U);
+  void test_g() { g(X{}, 0); }
+}

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp Wed Feb 15 21:49:44 2017
@@ -442,17 +442,13 @@ namespace dependent_nested_partial_speci
 namespace nondependent_default_arg_ordering {
   int n, m;
   template<typename A, A B = &n> struct X {};
-  template<typename A> void f(X<A>); // expected-note {{candidate}}
-  template<typename A> void f(X<A, &m>); // expected-note {{candidate}}
-  template<typename A, A B> void f(X<A, B>); // expected-note 2{{candidate}}
-  template<template<typename U, U> class T, typename A, int *B> void f(T<A, B>); // expected-note 2{{candidate}}
+  template<typename A> void f(X<A>);
+  template<typename A> void f(X<A, &m>);
+  template<typename A, A B> void f(X<A, B>) = delete; // expected-warning {{C++11}}
+  template<template<typename U, U> class T, typename A, int *B> void f(T<A, B>) = delete; // expected-warning {{C++11}}
   void g() {
-    // FIXME: The first and second function templates above should be
-    // considered more specialized than the last two, but during partial
-    // ordering we fail to check that we actually deduced template arguments
-    // that make the deduced A identical to A.
-    X<int *, &n> x; f(x); // expected-error {{ambiguous}}
-    X<int *, &m> y; f(y); // expected-error {{ambiguous}}
+    X<int *, &n> x; f(x);
+    X<int *, &m> y; f(y);
   }
 }
 

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp?rev=295277&r1=295276&r2=295277&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp Wed Feb 15 21:49:44 2017
@@ -328,3 +328,23 @@ namespace Nested {
   void g(int, int);
   using Int = A<int>::B<&g>::param2;
 }
+
+namespace nondependent_default_arg_ordering {
+  int n, m;
+  template<typename A, A B = &n> struct X {};
+  template<typename A> void f(X<A>); // #1, expected-note {{candidate}}
+  template<typename A> void f(X<A, &m>); // #2, expected-note {{candidate}}
+  template<typename A, A B> void f(X<A, B>); // #3, expected-note 2{{candidate}}
+  template<template<typename U, U> class T, typename A, int *B> void f(T<A, B>); // #4, expected-note 2{{candidate}}
+  void g() {
+    // These become ill-formed in C++1z because we can now deduce the
+    // type A in #3 and #4 in two ways during partial ordering:
+    //  * we can deduce #3's A = #1's A from the template-id
+    //  * we can deduce #3's A = 'int *' from the type of the value
+    //    deduced as #3's B
+    // FIXME: It seems unfortunate that we have to reject this; #1 and #2 are
+    // obviously more specialized than #3 and #4.
+    X<int *, &n> x; f(x); // expected-error {{ambiguous}}
+    X<int *, &m> y; f(y); // expected-error {{ambiguous}}
+  }
+}




More information about the cfe-commits mailing list