Customize the SFINAE diagnostics for enable_if to provide the failed condition.

[doug.gregor]

Summary

When enable_if disables a particular overload resolution candidate,
rummage through the enable_if condition to find the specific condition
that caused the failure. For example, if we have something like:

template<
  typename Iter,
  typename = std::enable_if_t<Random_access_iterator<Iter> &&
                              Comparable<Iterator_value_type<Iter>>>>
void mysort(Iter first, Iter last) {}

and we call "mysort" with "std::list<int>" iterators, we'll get a
diagnostic saying that the "Random_access_iterator<Iter>" requirement
failed. If we call "mysort" with
"std::vector<something_not_comparable>", we'll get a diagnostic saying
that the "Comparable<...>" requirement failed.

[rsmith]

I don't especially like that we pretty-print the substituted subexpression to name the requirement, but I'm also not sure any other alternative would be better.

I think the "tell me why this expression evaluates to false" mechanism really belongs in the constant expression evaluator -- I have a few things I'd like to add to it (recursion through ||, hints about the values of multiple subexpressions that contribute to the overall result, the values of operands of == and !=, and so on) and then I'd like to use this mechanism to better diagnose static_assert failures -- but this looks fine for now. I'm happy to move / refactor it later for improved static_assert diagnosis.

lib/Sema/SemaTemplate.cpp

2838–2852

I can imagine this being a bit surprising in other cases where it also fires (eg enable_if<N == 3 || N == 4> would only diagnose the N == 4 part). If we're only targeting the precise pattern used by ranges v3 here, we could check that the expression comes from a macro named CONCEPT_REQUIRES or CONCEPT_REQUIRES_.

[doug.gregor]

lib/Sema/SemaTemplate.cpp

2838–2852

That's a good point; I could try to recognize the complete idiom (defaulted non type template parameter and all), but it's more straightforward to check the macro name.

[doug.gregor]

Yes, digging into a static_assert to determine which condition caused it to fail would be quite useful QoI, and it should be fairly easy to use this mechanism to do that. We might want to dig one level deeper to give more information, e.g., static_assert(Comparable<T>) could look into the definition of Comparable<T> to see which piece of it failed.

Moving this into the constant expression evaluator would let us produce more specific diagnostics, although I think we get the most benefit from this patch and the extension to static_assert.

[doug.gregor]

Committed as r307196 and r307197

[eric_niebler]

lib/Sema/SemaTemplate.cpp

2838–2852

Note to self: never change the names of the CONCEPT_REQUIRES macros. ;-) Thanks for this, BTW!

include/clang/Basic/DiagnosticSemaKinds.td

     "candidate template ignored: substitution failure%0%1">;
 def note_ovl_candidate_disabled_by_enable_if : Note<
     "candidate template ignored: disabled by %0%1">;
+def note_ovl_candidate_disabled_by_requirement : Note<
+    "candidate template ignored: requirement '%0' was not satisfied%1">;
 def note_ovl_candidate_has_pass_object_size_params: Note<
     "candidate address cannot be taken because parameter %0 has "
     "pass_object_size attribute">;

 def err_typename_nested_not_found_enable_if : Error<
   "no type named 'type' in %0; 'enable_if' cannot be used to disable "
   "this declaration">;
+def err_typename_nested_not_found_requirement : Error<
+  "failed requirement '%0'; 'enable_if' cannot be used to disable this "
+  "declaration">;
 def err_typename_nested_not_type : Error<
     "typename specifier refers to non-type member %0 in %1">;
 def note_typename_refers_here : Note<

include/clang/Basic/PartialDiagnostic.h

 
   bool hasStorage() const { return DiagStorage != nullptr; }
 
+  /// Retrieve the string argument at the given index.
+  StringRef getStringArg(unsigned I) {
+    assert(DiagStorage && "No diagnostic storage?");
+    assert(I < DiagStorage->NumDiagArgs && "Not enough diagnostic args");
+    assert(DiagStorage->DiagArgumentsKind[I]
+             == DiagnosticsEngine::ak_std_string && "Not a string arg");
+    return DiagStorage->DiagArgumentsStr[I];
+  }
+
   friend const PartialDiagnostic &operator<<(const PartialDiagnostic &PD,
                                              unsigned I) {
     PD.AddTaggedVal(I, DiagnosticsEngine::ak_uint);

include/clang/Sema/TemplateDeduction.h

     HasSFINAEDiagnostic = false;
   }
 
+  /// Peek at the SFINAE diagnostic.
+  const PartialDiagnosticAt &peekSFINAEDiagnostic() const {
+    assert(HasSFINAEDiagnostic);
+    return SuppressedDiagnostics.front();
+  }
+
   /// \brief Provide a new template argument list that contains the
   /// results of template argument deduction.
   void reset(TemplateArgumentList *NewDeduced) {

lib/Sema/SemaOverload.cpp

       return;
     }
 
+    // We found a specific requirement that disabled the enable_if.
+    if (PDiag && PDiag->second.getDiagID() ==
+        diag::err_typename_nested_not_found_requirement) {
+      S.Diag(Templated->getLocation(),
+             diag::note_ovl_candidate_disabled_by_requirement)
+        << PDiag->second.getStringArg(0) << TemplateArgString;
+      return;
+    }
+
     // Format the SFINAE diagnostic into the argument string.
     // FIXME: Add a general mechanism to include a PartialDiagnostic *'s
     //        formatted message in another diagnostic.

lib/Sema/SemaTemplate.cpp

   llvm_unreachable("unexpected BuiltinTemplateDecl!");
 }
 
+/// Determine whether this alias template is "enable_if_t".
+static bool isEnableIfAliasTemplate(TypeAliasTemplateDecl *AliasTemplate) {
+  return AliasTemplate->getName().equals("enable_if_t");
+}
+
+/// Collect all of the separable terms in the given condition, which
+/// might be a conjunction.
+///
+/// FIXME: The right answer is to convert the logical expression into
+/// disjunctive normal form, so we can find the first failed term
+/// within each possible clause.
+static void collectConjunctionTerms(Expr *Clause,
+                                    SmallVectorImpl<Expr *> &Terms) {
+  if (auto BinOp = dyn_cast<BinaryOperator>(Clause->IgnoreParenImpCasts())) {
+    if (BinOp->getOpcode() == BO_LAnd) {
+      collectConjunctionTerms(BinOp->getLHS(), Terms);
+      collectConjunctionTerms(BinOp->getRHS(), Terms);
+    }
+
+    return;
+  }
+
+  Terms.push_back(Clause);
+}
+
+/// Find the failed subexpression within enable_if, and describe it
+/// with a string.
+static std::pair<Expr *, std::string>
+findFailedEnableIfCondition(Sema &S, Expr *Cond) {
+  // If the condition is an "or" with a silly first term, ignore the
+  // first term. This is effective pattern-matching the
+  // CONCEPT_REQUIRES_ hack in Ranges v3.
+  if (auto BinOp = dyn_cast<BinaryOperator>(Cond->IgnoreParenImpCasts())) {
+    if (BinOp->getOpcode() == BO_LOr) {
+      Expr *LHS = BinOp->getLHS();
+      if (auto InnerBinOp =
+            dyn_cast<BinaryOperator>(LHS->IgnoreParenImpCasts())) {
+        if (InnerBinOp->getOpcode() == BO_EQ &&
+            isa<IntegerLiteral>(InnerBinOp->getRHS())) {
+          Cond = BinOp->getRHS();
+        }
+      }
+    }
+  }
+
+  // Separate out all of the terms in a conjunction.
+  SmallVector<Expr *, 4> Terms;
+  collectConjunctionTerms(Cond, Terms);
+
+  // Determine which term failed.
+  Expr *FailedCond = nullptr;
+  for (Expr *Term : Terms) {
+    // The initialization of the parameter from the argument is
+    // a constant-evaluated context.
+    EnterExpressionEvaluationContext ConstantEvaluated(
+      S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+
+    bool Succeeded;
+    if (Term->EvaluateAsBooleanCondition(Succeeded, S.Context) &&
+        !Succeeded) {
+      FailedCond = Term->IgnoreParenImpCasts();
+      break;
+    }
+  }
+
+  if (!FailedCond)
+    FailedCond = Cond->IgnoreParenImpCasts();
+
+  std::string Description;
+  {
+    llvm::raw_string_ostream Out(Description);
+    FailedCond->printPretty(Out, nullptr,
+                            PrintingPolicy(S.Context.getLangOpts()));
+  }
+  return { FailedCond, Description };
+}
+
 QualType Sema::CheckTemplateIdType(TemplateName Name,
                                    SourceLocation TemplateLoc,
                                    TemplateArgumentListInfo &TemplateArgs) {

     if (Pattern->isInvalidDecl())
       return QualType();
 
-    TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack,
+    TemplateArgumentList StackTemplateArgs(TemplateArgumentList::OnStack,
                                            Converted);
 
     // Only substitute for the innermost template argument list.
     MultiLevelTemplateArgumentList TemplateArgLists;
-    TemplateArgLists.addOuterTemplateArguments(&TemplateArgs);
+    TemplateArgLists.addOuterTemplateArguments(&StackTemplateArgs);
     unsigned Depth = AliasTemplate->getTemplateParameters()->getDepth();
     for (unsigned I = 0; I < Depth; ++I)
       TemplateArgLists.addOuterTemplateArguments(None);

     CanonType = SubstType(Pattern->getUnderlyingType(),
                           TemplateArgLists, AliasTemplate->getLocation(),
                           AliasTemplate->getDeclName());
-    if (CanonType.isNull())
+    if (CanonType.isNull()) {
+      // If this was enable_if and we failed to find the nested type
+      // within enable_if in a SFINAE context, dig out the specific
+      // enable_if condition that failed and present that instead.
+      if (isEnableIfAliasTemplate(AliasTemplate)) {
+        if (auto DeductionInfo = isSFINAEContext()) {
+          if (*DeductionInfo &&
+              (*DeductionInfo)->hasSFINAEDiagnostic() &&
+              (*DeductionInfo)->peekSFINAEDiagnostic().second.getDiagID() ==
+                diag::err_typename_nested_not_found_enable_if &&
+              TemplateArgs[0].getArgument().getKind()
+                == TemplateArgument::Expression) {
+            Expr *FailedCond;
+            std::string FailedDescription;
+            std::tie(FailedCond, FailedDescription) =
+              findFailedEnableIfCondition(
+                *this, TemplateArgs[0].getSourceExpression());
+
+            // Remove the old SFINAE diagnostic.
+            PartialDiagnosticAt OldDiag =
+              {SourceLocation(), PartialDiagnostic::NullDiagnostic()};
+            (*DeductionInfo)->takeSFINAEDiagnostic(OldDiag);
+
+            // Add a new SFINAE diagnostic specifying which condition
+            // failed.
+            (*DeductionInfo)->addSFINAEDiagnostic(
+              OldDiag.first,
+              PDiag(diag::err_typename_nested_not_found_requirement)
+                << FailedDescription
+                << FailedCond->getSourceRange());
+          }
+        }
+      }
+
       return QualType();
+    }
   } else if (Name.isDependent() ||
              TemplateSpecializationType::anyDependentTemplateArguments(
                TemplateArgs, InstantiationDependent)) {

 /// Determine whether this failed name lookup should be treated as being
 /// disabled by a usage of std::enable_if.
 static bool isEnableIf(NestedNameSpecifierLoc NNS, const IdentifierInfo &II,
-                       SourceRange &CondRange) {
+                       SourceRange &CondRange, Expr *&Cond) {
   // We must be looking for a ::type...
   if (!II.isStr("type"))
     return false;

 
   // Assume the first template argument is the condition.
   CondRange = EnableIfTSTLoc.getArgLoc(0).getSourceRange();
+
+  // Dig out the condition.
+  Cond = nullptr;
+  if (EnableIfTSTLoc.getArgLoc(0).getArgument().getKind()
+        != TemplateArgument::Expression)
+    return true;
+
+  Cond = EnableIfTSTLoc.getArgLoc(0).getSourceExpression();
+
+  // Ignore Boolean literals; they add no value.
+  if (isa<CXXBoolLiteralExpr>(Cond->IgnoreParenCasts()))
+    Cond = nullptr;
+
   return true;
 }
 

     // If we're looking up 'type' within a template named 'enable_if', produce
     // a more specific diagnostic.
     SourceRange CondRange;
-    if (isEnableIf(QualifierLoc, II, CondRange)) {
+    Expr *Cond = nullptr;
+    if (isEnableIf(QualifierLoc, II, CondRange, Cond)) {
+      // If we have a condition, narrow it down to the specific failed
+      // condition.
+      if (Cond) {
+        Expr *FailedCond;
+        std::string FailedDescription;
+        std::tie(FailedCond, FailedDescription) =
+          findFailedEnableIfCondition(*this, Cond);
+
+        Diag(FailedCond->getExprLoc(),
+             diag::err_typename_nested_not_found_requirement)
+          << FailedDescription
+          << FailedCond->getSourceRange();
+        return QualType();
+      }
+
       Diag(CondRange.getBegin(), diag::err_typename_nested_not_found_enable_if)
           << Ctx << CondRange;
       return QualType();
     }
 

test/SemaTemplate/constexpr-instantiate.cpp

       static constexpr bool f() { return sizeof(T) < U::size; }
 
       template<typename U>
-      static typename enable_if<f<U>(), void>::type g() {} // expected-note {{disabled by 'enable_if'}}
+      static typename enable_if<f<U>(), void>::type g() {} // expected-note {{requirement 'f<Unevaluated::PR13423::U>()' was not satisfied}}
     };
 
     struct U { static constexpr int size = 2; };

test/SemaTemplate/overload-candidates.cpp

   template<bool, typename = void> struct enable_if {};
   template<typename T> struct enable_if<true, T> { typedef T type; };
 }
-template<typename T> typename boost::enable_if<sizeof(T) == 4, int>::type if_size_4(); // expected-note{{candidate template ignored: disabled by 'enable_if' [with T = char]}}
+template<typename T> typename boost::enable_if<sizeof(T) == 4, int>::type if_size_4(); // expected-note{{candidate template ignored: requirement 'sizeof(char) == 4' was not satisfied [with T = char]}}
 int k = if_size_4<char>(); // expected-error{{no matching function}}
 
 namespace llvm {

 }
 
 template<typename T> struct NonTemplateFunction {
-  typename boost::enable_if<sizeof(T) == 4, int>::type f(); // expected-error{{no type named 'type' in 'boost::enable_if<false, int>'; 'enable_if' cannot be used to disable this declaration}}
+  typename boost::enable_if<sizeof(T) == 4, int>::type f(); // expected-error{{failed requirement 'sizeof(char) == 4'; 'enable_if' cannot be used to disable this declaration}}
 };
 NonTemplateFunction<char> NTFC; // expected-note{{here}}
 

 #if __cplusplus <= 199711L
   // expected-warning@-2 {{default template arguments for a function template are a C++11 extension}}
 #endif
-  // expected-note@-4 {{candidate template ignored: disabled by 'enable_if' [with T = int]}}
+  // expected-note@+1 {{candidate template ignored: requirement 'a_trait<int>::value' was not satisfied [with T = int]}}
   void foo() {}
   void bar() { foo<int>(); } // expected-error {{no matching function for call to 'foo'}}
 

 #if __cplusplus <= 199711L
   // expected-warning@-2 {{alias declarations are a C++11 extension}}
 #endif
-  // expected-note@-4 {{candidate template ignored: disabled by 'enable_if' [with T = int]}}
+  // expected-note@+7 {{candidate template ignored: requirement 'some_trait<int>::value' was not satisfied [with T = int]}}
 
   template<typename T,
            typename Requires = unicorns<T> >

 #endif
   void wibble() {}
   void wobble() { wibble<int>(); } // expected-error {{no matching function for call to 'wibble'}}
+
+  template<typename T>
+  struct some_passing_trait : std::true_type {};
+
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{default template arguments for a function template are a C++11 extension}}
+  // expected-warning@+4 {{default template arguments for a function template are a C++11 extension}}
+#endif
+  template<typename T,
+           int n = 42,
+           typename std::enable_if<n == 43 || (some_passing_trait<T>::value && some_trait<T>::value), int>::type = 0>
+  void rangesv3(); // expected-note{{candidate template ignored: requirement 'some_trait<int>::value' was not satisfied [with T = int, n = 42]}}
+  void test_rangesv3() { rangesv3<int>(); } // expected-error{{no matching function for call to 'rangesv3'}}
 }