[PATCH] Restrict -Winconsistent-missing-override warnings on class templates with dependent bases

[David Blaikie]

On Fri, Mar 6, 2015 at 2:57 PM, jahanian <fjahanian at apple.com> wrote:

>
> On Mar 6, 2015, at 2:47 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Mar 6, 2015 at 2:45 PM, jahanian <fjahanian at apple.com> wrote:
>
>>
>> On Mar 6, 2015, at 9:36 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Would it be plausible to check this on templates directly, rather than on
>> their instantiations? This would be less work in the case of multiple
>> instantiations, avoid redundant diagnostics, fail on templates without
>> instantiations rather than creating a lurking failure, and we might even
>> get all the "dependent" tests for free - because we wouldn't be able to
>> look through the dependent types at all.
>>
>>
>> It could be plausible. But, in similar cases, checking is done on the
>> instantiated templates and not on the templates directly. This adds another
>> check in the
>> same code block. Providing a new iteration on templates for this one
>> check is prohibitively expensive (and we normally don’t do much checking on
>> templates).
>>
>
> It is? I'd be curious to see the numbers, as it sounds like you have some.
>
>
> No I don’t have any. But iterating over templates looking for methods
> would add to cost.
>

Well, if we skipped template instantiations it wouldn't necessarily be a
strict increase - we'd only visit the template pattern, and none of the
instantiations.


> Are we iterating over templates for other diagnostics?
>
If so, I can add this there.
>

I don't recall any particular cases for/against.


>
>
>
>> Do you see anything inherently wrong to adding this check where it is?
>>
>
> Just the issues I mentioned - duplicate diagnostics in the case of
> multiple instantiations (& no diagnostics in the case of no instantiations).
>
>
> AFAIK, it is not common practice to add diagnostics on template
> declarations as this will break SDKs.
>

Not sure I follow - any type in a header/library could be violating this
rule, whether it's a class template or a straight class.


> Furthermore, if template is not used
> in practice, there is no point issuing this diagnostic.
>

Same would be true of non-template classes in headers, but we don't avoid
those.


> Can you point to similar situations where templates are iterated over
> checking for method
> inconsistencies or other types of diagnostics?
>

Not that I recall/off-hand.


I do recall Ted objecting to my desire to move unreachable-code warnings to
act on template patterns rather than template instantiations  (& he didn't
have numbers either, but the case is different - in that case there are
many more function template definitions in headers that aren't necessarily
instantiated (owing to the language need for the definitions to be
available for instantiations) - unlike classes V class templates, which
just appear in headers in a fairly similar way, not a particularly biased
ratio one way or the other (except by library design preferences))

- David


>
> - Fariborz
>
>
>
>>
>> - Fariborz
>>
>>
>> - David
>>
>> On Wed, Feb 25, 2015 at 10:39 AM, jahanian <fjahanian at apple.com> wrote:
>>
>>>
>>> This patch restricts issuing -Winconsistent-missing-override when
>>> dealing with
>>> class template with dependent bases and dependent methods.
>>> Fixed pr22582 rdar://19917107.
>>> Please review.
>>>
>>> - Fariborz
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150306/119bfdd9/attachment.html>