Fix enable_if evaluation in value-dependent contexts.

[george.burgess.iv]

Summary

This patch is meant to be applied instead of D18425, but is a new review because we're taking an entirely different approach.

This patch makes us fail enable_if conditions in a value dependent context, if not all values are present when we're trying to evaluate the enable_if. e.g.

void foo(int i) __attribute__((enable_if(i, "")));
template <int N> void callFoo() { foo(N); } // Error: No viable overload (emitted before any instantiations of callFoo are made), because the call to foo is resolved prior to instantiating callFoo, so we don't have a single value for N.

Our current behavior is that we'll happily resolve foo to the only existing foo overload, and be totally fine with callFoo<0>(); as a result, which is incorrect. Were there two foo overloads with different enable_if attributes, we would always emit an error about an ambiguous overload set.

A complete, consistent fix for this would require that we're able to arbitrarily defer overload resolution. Additionally, we would need to handle type dependence appearing out of nowhere, as in:

double bar(int N) __attribute__((enable_if(N, "")));
void *bar(int N) __attribute__((enable_if(!N, "")));
template <int N>
auto callBar() { return bar(N); }

Which seems like a very large, bug-prone change.

Given that it seems no one has tried to use enable_if in a value-dependent context yet (if they were, I'd assume we would have received a bug report about bar(N) always being ambiguous by now), I think this is the least bad of all of our options.

cfe/trunk/include/clang/Basic/AttrDocs.td

.. code-block:: c
   void fn() {
     int (*ptr)(int);
     ptr = &f; // error: 'a > 0' is not always true
     ptr = &g; // error: 'a == 0 || a != 0' is not a truthy constant
     ptr = &h; // OK: 1 is a truthy constant
     ptr = &i; // OK: 'TrueConstant' is a truthy constant
     ptr = &j; // error: 'FalseConstant' is a constant, but not truthy
   }
+
+Because ``enable_if`` evaluation happens during overload resolution,
+``enable_if`` may give unintuitive results when used with templates, depending
+on when overloads are resolved. In the example below, clang will emit a
+diagnostic about no viable overloads for ``foo`` in ``bar``, but not in ``baz``:
+
+.. code-block:: c++
+
+  double foo(int i) __attribute__((enable_if(i > 0, "")));
+  void *foo(int i) __attribute__((enable_if(i <= 0, "")));
+  template <int I>
+  auto bar() { return foo(I); }
+
+  template <typename T>
+  auto baz() { return foo(T::number); }
+
+  struct WithNumber { constexpr static int number = 1; };
+  void callThem() {
+    bar<sizeof(WithNumber)>();
+    baz<WithNumber>();
+  }
+
+This is because, in ``bar``, ``foo`` is resolved prior to template
+instantiation, so the value for ``I`` isn't known (thus, both ``enable_if``
+conditions for ``foo`` fail). However, in ``baz``, ``foo`` is resolved during
+template instantiation, so the value for ``T::number`` is known.
   }];
 }
 
 def PassObjectSizeDocs : Documentation {
   let Category = DocCatVariable; // Technically it's a parameter doc, but eh.
   let Content = [{
 .. Note:: The mangling of functions with parameters that are annotated with
   ``pass_object_size`` is subject to change. You can get around this by

cfe/trunk/lib/Sema/SemaOverload.cpp

EnableIfAttr *Sema::CheckEnableIf(FunctionDecl *Function, ArrayRef<Expr *> Args,
                                   bool MissingImplicitThis) {
   auto EnableIfAttrs = getOrderedEnableIfAttrs(Function);
   if (EnableIfAttrs.empty())
     return nullptr;
 
   SFINAETrap Trap(*this);
   SmallVector<Expr *, 16> ConvertedArgs;
   bool InitializationFailed = false;
-  bool ContainsValueDependentExpr = false;
 
   // Convert the arguments.
   for (unsigned I = 0, E = Args.size(); I != E; ++I) {
     ExprResult R;
     if (I == 0 && !MissingImplicitThis && isa<CXXMethodDecl>(Function) &&
         !cast<CXXMethodDecl>(Function)->isStatic() &&
         !isa<CXXConstructorDecl>(Function)) {
       CXXMethodDecl *Method = cast<CXXMethodDecl>(Function);
       R = PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/nullptr,
                                               Method, Method);
     } else {
       R = PerformCopyInitialization(InitializedEntity::InitializeParameter(
                                         Context, Function->getParamDecl(I)),
                                     SourceLocation(), Args[I]);
     }
 
     if (R.isInvalid()) {
       InitializationFailed = true;
       break;
     }
 
-    ContainsValueDependentExpr |= R.get()->isValueDependent();
     ConvertedArgs.push_back(R.get());
   }
 
   if (InitializationFailed || Trap.hasErrorOccurred())
     return EnableIfAttrs[0];
 
   // Push default arguments if needed.
   if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
     for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
       ParmVarDecl *P = Function->getParamDecl(i);
       ExprResult R = PerformCopyInitialization(
           InitializedEntity::InitializeParameter(Context,
                                                  Function->getParamDecl(i)),
           SourceLocation(),
           P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
                                            : P->getDefaultArg());
       if (R.isInvalid()) {
         InitializationFailed = true;
         break;
       }
-      ContainsValueDependentExpr |= R.get()->isValueDependent();
       ConvertedArgs.push_back(R.get());
     }
 
     if (InitializationFailed || Trap.hasErrorOccurred())
       return EnableIfAttrs[0];
   }
 
   for (auto *EIA : EnableIfAttrs) {
     APValue Result;
-    if (EIA->getCond()->isValueDependent()) {
-      // Don't even try now, we'll examine it after instantiation.
-      continue;
-    }
-
+    // FIXME: This doesn't consider value-dependent cases, because doing so is
+    // very difficult. Ideally, we should handle them more gracefully.
     if (!EIA->getCond()->EvaluateWithSubstitution(
-            Result, Context, Function, llvm::makeArrayRef(ConvertedArgs))) {
-      if (!ContainsValueDependentExpr)
-        return EIA;
-    } else if (!Result.isInt() || !Result.getInt().getBoolValue()) {
+            Result, Context, Function, llvm::makeArrayRef(ConvertedArgs)))
+
+    if (!Result.isInt() || !Result.getInt().getBoolValue())
       return EIA;
-    }
-  }
   return nullptr;
 }
 
 /// \brief Add all of the function declarations in the given function set to
 /// the overload candidate set.
 void Sema::AddFunctionCandidates(const UnresolvedSetImpl &Fns,
                                  ArrayRef<Expr *> Args,
                                  OverloadCandidateSet& CandidateSet,

cfe/trunk/test/SemaCXX/enable_if.cpp

void test() {
   typedep(n);  // expected-note{{in instantiation of function template specialization 'typedep<Nothing>' requested here}}
 }
 
 template <typename T> class C {
   void f() __attribute__((enable_if(T::expr == 0, ""))) {}
   void g() { f(); }
 };
 
-int fn3(bool b) __attribute__((enable_if(b, "")));
+int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
 template <class T> void test3() {
-  fn3(sizeof(T) == 1);
+  fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
 }
 
 template <typename T>
 struct Y {
   T h(int n, int m = 0) __attribute__((enable_if(m == 0, "chosen when 'm' is zero")));  // expected-note{{candidate disabled: chosen when 'm' is zero}}
 };
 
 void test4() {
   Y<int> y;
 
   int t0 = y.h(0);
   int t1 = y.h(1, 2);  // expected-error{{no matching member function for call to 'h'}}
 }
 
 // FIXME: issue an error (without instantiation) because ::h(T()) is not
 // convertible to bool, because return types aren't overloadable.
 void h(int);
 template <typename T> void outer() {
   void local_function() __attribute__((enable_if(::h(T()), "")));
-  local_function();
+  local_function(); // expected-error{{no matching function for call to 'local_function'}} expected-note@-1{{candidate disabled}}
 };
 
 namespace PR20988 {
   struct Integer {
     Integer(int);
   };
 
   int fn1(const Integer &) __attribute__((enable_if(true, "")));
   template <class T> void test1() {
     int &expr = T::expr();
     fn1(expr);
   }
 
   int fn2(const Integer &) __attribute__((enable_if(false, "")));  // expected-note{{candidate disabled}}
   template <class T> void test2() {
     int &expr = T::expr();
     fn2(expr);  // expected-error{{no matching function for call to 'fn2'}}
   }
 
-  int fn3(bool b) __attribute__((enable_if(b, "")));
+  int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
   template <class T> void test3() {
-    fn3(sizeof(T) == 1);
+    fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
   }
 }
 
 namespace FnPtrs {
   int ovlFoo(int m) __attribute__((enable_if(m > 0, "")));
   int ovlFoo(int m);
 
   void test() {

 };
 
 void runFoo() {
   Foo f;
   f.bar(); // expected-error{{too few arguments}}
   f.bar(1, 2); // expected-error{{too many arguments}}
 }
 }
+
+// Ideally, we should be able to handle value-dependent expressions sanely.
+// Sadly, that isn't the case at the moment.
+namespace dependent {
+int error(int N) __attribute__((enable_if(N, ""))); // expected-note{{candidate disabled}}
+int error(int N) __attribute__((enable_if(!N, ""))); // expected-note{{candidate disabled}}
+template <int N> int callUnavailable() {
+  return error(N); // expected-error{{no matching function for call to 'error'}}
+}
+
+constexpr int noError(int N) __attribute__((enable_if(N, ""))) { return -1; }
+constexpr int noError(int N) __attribute__((enable_if(!N, ""))) { return -1; }
+constexpr int noError(int N) { return 0; }
+
+template <int N>
+constexpr int callNoError() { return noError(N); }
+static_assert(callNoError<0>() == 0, "");
+static_assert(callNoError<1>() == 0, "");
+
+template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
+  return 1;
+}
+
+constexpr int A = templated<0>(); // expected-error{{no matching function for call to 'templated'}} expected-note@-4{{candidate disabled}}
+static_assert(templated<1>() == 1, "");
+
+template <int N> constexpr int callTemplated() { return templated<N>(); }
+
+constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+static_assert(callTemplated<1>() == 1, "");
+}