Page MenuHomePhabricator

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

Authored by doug.gregor on Jul 4 2017, 11:26 PM.

Details

Reviewers
rsmith
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.

Diff Detail

Event Timeline

doug.gregor created this revision.Jul 4 2017, 11:26 PM
rsmith accepted this revision.Jul 5 2017, 1:26 AM

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_.

This revision is now accepted and ready to land.Jul 5 2017, 1:26 AM
doug.gregor added inline comments.Jul 5 2017, 9:16 AM
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.

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 closed this revision.Jul 5 2017, 1:22 PM

Committed as r307196 and r307197

eric_niebler added inline comments.
lib/Sema/SemaTemplate.cpp
2838–2852

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