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

[Douglas Gregor]

On Jun 8, 2010, at 1:59 PM, Jeffrey Yasskin wrote:

> 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?

So long as the default remains "all" until we have decent heuristics for a quality-based cutoff, I'm happy to have support for "best" in the tree with the 4-overload cutoff.

	- Doug