patch: improve handling of value dependent exprs in CheckEnableIf

[Nick Lewycky]

On 15 December 2014 at 14:25, Richard Smith <richard at metafoo.co.uk> wrote:
>
> +    if (EIA->getCond()->isValueDependent()) {
> +      // Don't even try now, we'll examine it after instantiation.
> +      continue;
> +    }
>
> Is this sufficient? Suppose we have:
>
> template<bool B> struct S {
>   void f() __attribute__((enable_if(B, "")));
>   void f() __attribute__((enable_if(!B, "")));
>   void g() { f(); }
> };
>
> Will we reject the template due to the amiguity in overload resolution for
> f()? (If not, what saves us?)
>

We do not. With just this TU, we never even get into overload resolution.
Sema::ActOnCallExpr saves us by bailing very early producing a dependent
CallExpr before even trying to build a member function call, because g()'s
type is type dependent. See the comment "Determine whether this is a
dependent call inside a C++ template, in which case we won't do any
semantic analysis now" in SemaExpr.cpp.

If it had gone through AddOverloadCandidate, it would call CheckEnableIf
which would return that no enable_if expression had failed, and we would
build up an overload candidate set with two equally ranked candidates, but
that isn't an error yet. Whoever would be doing that would also need to
handle the case of:

  void f(X);
  void f(Y);
  template<typename T> void g() { f(T()); }

Likewise for:
>
> template<bool B> struct S {
>   void f(bool b) __attribute__((enable_if(b, "")));
>   void f(bool b) __attribute__((enable_if(!b, "")));
>   void g() { f(B); }
> };
>

Again no error for the same reasons.

+// 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();
> +};
>
> This FIXME doesn't seem correct: a different 'h' could be found by ADL,
> and it could be a constexpr function returning bool. What happens if you
> use ::h instead?
>

Good catch! I had forgotten about ADL here.

I get the same thing with ::h, also (h) and (::h). I'll update the code and
comment to use ::h.

Nick

LGTM with the FIXME corrected; we can fix the semantics of the dependent
> case (if it's not working) after the crash is fixed.
>
> On Mon, Dec 8, 2014 at 8:43 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
>> The attached patch improves our handling of value dependent expressions
>> in attribute((enable_if)), both in the condition expression and at the call
>> site. Fixes PR20988.
>>
>> Please review!
>>
>> Nick
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141215/37be50c6/attachment.html>