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

[Jeffrey Yasskin]

On Tue, Jun 8, 2010 at 2:01 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> 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.
>

Will do. Thanks!