[cfe-commits] [PATCH] Limit the number of overload candidates printed (issue1591041)

[Jeffrey Yasskin]

On Tue, Jun 8, 2010 at 1:52 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Jun 8, 2010, at 1:06 PM, Jeffrey Yasskin wrote:
>
> On Tue, Jun 8, 2010 at 7:49 AM, Douglas Gregor <dgregor at apple.com> wrote:
>>
>> On Jun 7, 2010, at 6:03 PM, jyasskin at gmail.com wrote:
>>
>>> Reviewers: cfe-commits_cs.uiuc.edu,
>>>
>>> Message:
>>> Please take a look. If you prefer reviewing diffs, they're behind the
>>> "Download raw patch set" link.
>>>
>>> Description:
>>> When there are lots of operator<<s, clang produces significantly worse
>>> diagnostics than gcc, simply because of the size of the output. This
>>> patch limits clang to 4 overload candidates, with the ability to show
>>> the rest by passing -fshow-all-overloads, as a first cut. We'll want to
>>> refine that later as examples of bad behavior come up.
>>
>>
>> Unless we can be fairly sure that the "right" operator<< is in those first
>> 4 overload candidates, I don't think this is a good idea. Unlike with
>> suppressing inner template/macro instantiation histories, this change is
>> likely to suppress important information.
>>
>
> I agree that it will sometimes suppress important information. That's why I
> added the -fshow-all-overloads flag so the user can get it back if they need
> it.
>
> Sure, and it's good to have -fshow-all-overloads for any kind of pruning. My
> concern is that if the pruning is not good by default, we'll end up causing
> more harm than good: the user will have to bounce between
> -fshow-all-overloads and non-fshow-all-overloads whenever they hit problems.
> That's worse than having a longer diagnostic chain in the first place.
>
> But in cases like the one below, there are too many overloads printed to
> find the "right" one, even if it were present, and they just discourage
> users from reading any of them. 4 is clearly not the right cut-off in all
> cases, and cutting off after a drop in quality is likely to be better in
> many cases, but it fixes some of the most egregious behavior pretty easily.
> We can fix places where it omits useful overloads as they come up.
>
> We can, but our heuristics are known not to be that good, so we won't even
> have a good sense of how useful this change is until we have better
> heuristics.
>
> If you prefer, I can look for a quality drop based
> on CompareOverloadCandidatesForDisplay instead of the fixed cutoff. I'll
> want a hard cutoff around 6-10 anyway, since at that point I think most
> users give up on our errors and just stare at the source instead.
>
> I think a quality-based cutoff is the only workable solution, so IMO we need
> that before we can turn this behavior on by default.
> It would probably make sense to have the flag set
> -fshow-overloads={best,all}
> so that we have the option later of adding different tweaks/heuristics
> (e.g., "detailed", to really show what happens for each overload).
> Otherwise, we'll end up with several -fshow-*-overloads flags.

That does sound better. Would you accept a -fshow-overloads={best,all}
that defaulted to 'all' and had 'best' do the 4-overload cutoff, or
would you want 'best' to look for a quality change in the first
version?