[PATCH] Improve error reporting for SFINAE

[Richard Smith]

On Mon, Apr 6, 2015 at 4:16 PM, paul Fultz <pfultz2 at yahoo.com> wrote:

> > OK, but that may involve changing system headers
>
> The current diagnostics may require changing system headers as well. In the
> real world it's not as simple as f -> g -> h. See
> [here](https://github.com/ericniebler/range-v3/issues/94) where the user
> modifies library headers to diagnosis the problem.
>
> > Nonetheless, I don't think you have evidence for your "90%" claim
>
> Granted I didn't do a complete survey, and measuring complaints is invalid
> since the complaints about the proposed feature are non-existent. However,
> from my personal experience using this patch, I have yet to see a need for
> full backtrace. There is probably is a need for it, but it is very small.
>
> > We know how to make all the cases better: if there's only one viable
> > candidate, provide a full instantiation backtrace for it -- why not do
> that
> > instead?
>
> That is my end goal. At least to have a compiler flag for it as well.
>

If possible, our default mode should be something that's "good enough" for
all cases (or at least, for as many cases as we can reasonably cover). So
I'd be inclined to say that our default mode should be to produce a
complete backtrace for the single-viable-function case.

However,
> it is a much larger task and requires possibly refactoring
> `DeductionFailureInfo` to store a complete trace. I don't work on clang
> full
> time so this will take awhile. So I figured with this patch, users can
> benefit
> from better error messages now.
>

Some will benefit; for others it will be a regression. One possible way
forward would be to put this behind a -cc1 flag so that people can
experiment with it and incremental work can continue on the complete trace.
Would that work for you?

Paul
>
>
>
>
>
>   On Monday, April 6, 2015 5:39 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>
>
> On Mon, Apr 6, 2015 at 3:06 PM, paul Fultz <pfultz2 at yahoo.com> wrote:
>
> > The difference is, the current diagnostics allow the user to figure out
> what
> > happened. This change does not -- there's no way to find out which 'f'
> caused
> > the problem, and how we got from 'f' to 'g'.
>
> Yes, the user can still figure out how you got from `f` to `g`. The user
> can
> either add an extra overload for `g` or comment out `g`(and continue so
> until
> the user gets to `f`). So instead of working forwards from `f` to `g`, the
> user just works backwards from `g` to `f`. However, its less likely a user
> would need to do that since 90% of the time users want to get to `g` and
> not
> to `f`.
>
>
> OK, but that may involve changing system headers, which is significantly
> more difficult in some cases. Nonetheless, I don't think you have evidence
> for your "90%" claim, so this would take us from one "not enough
> information is provided" case to another, which will make some cases better
> and others worse. We know how to make all the cases better: if there's only
> one viable candidate, provide a full instantiation backtrace for it -- why
> not do that instead?
>
> Paul
>
>
>
>
>   On Monday, April 6, 2015 4:24 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>
>
> On Mon, Apr 6, 2015 at 2:08 PM, Paul Fultz II <pfultz2 at yahoo.com> wrote:
>
> > gives the user no idea of how we got from a call to f into a call to g.
>
> For the default case, its better to show the first and last in the trace,
> as most users don't care about the intermediate steps.
>
> > If we produced a stack of 'in instantiation of' notes, this would be fine
>
> Yes, the full back trace can be added in the future(perhaps as a compiler
> flag). This is the first step towards that direction. There needs to be
> some refactoring to `DeductionFailureInfo` so it stores the entire
> diagnostics(rather than a single diagnostic) to make a full back trace
> possible.
>
>
> The difference is, the current diagnostics allow the user to figure out
> what happened. This change does not -- there's no way to find out which 'f'
> caused the problem, and how we got from 'f' to 'g'. Hence this patch is not
> acceptable as-is -- not even as a stepping stone to something better.
>
> > note that's exactly what my patch in comment#1/comment#2 of that bug does
>
> That seemed to be more of a hack then something to be used in production.
>
>
> Yes, absolutely, that change is not production-ready.
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150406/b7747c06/attachment.html>