<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 6, 2015 at 3:31 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Mar 6, 2015, at 3:15 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 6, 2015 at 2:57 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Mar 6, 2015, at 2:47 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 6, 2015 at 2:45 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Mar 6, 2015, at 9:36 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr">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.<br></div></div></blockquote><div><br></div></span>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</div><div>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).</div></div></blockquote><div><br>It is? I'd be curious to see the numbers, as it sounds like you have some.<br></div></div></div></div></div></blockquote><div><br></div></span>No I don’t have any. But iterating over templates looking for methods would add to cost.</div></div></blockquote><div><br>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.<br></div></div></div></div></div></blockquote><div><br></div></span>We already visit template instantiations for issuing other diagnostics. New overhead is minimal comparing to iterating over template declarations looking for methods.</div></div></blockquote><div><br>I don't quite follow - in this instance, given one instantiation of a template, we'd trade one visitation for another - the entirety of DiagnoseAbsenceOfOverrideControl would skip template instantiations and then be called for each template pattern.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> Are we iterating over templates for other diagnostics?</div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>If so, I can add this there.</div></div></blockquote><div><br>I don't recall any particular cases for/against.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Do you see anything inherently wrong to adding this check where it is?</div></div></blockquote><div><br>Just the issues I mentioned - duplicate diagnostics in the case of multiple instantiations (& no diagnostics in the case of no instantiations).<br></div></div></div></div></div></blockquote><div><br></div></span>AFAIK, it is not common practice to add diagnostics on template declarations as this will break SDKs.</div></div></blockquote><div><br>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.<br></div></div></div></div></div></blockquote><div><br></div></span>By this I meant that we don’t issue diagnostics on template declarations in other situations.</div></div></blockquote><div><br>Right, usually that's the right thing - for example, division by zero: we want to catch all the extra cases of div by zero that might happen only in specific instantiations of a template (eg: template<int n> void func() { x / n; } - if we used the template pattern we wouldn't see a div by zero, but by using the instantiation we can see div by zero in func<0>). It just happens to be in this instance it's the other way around - & it's the case for a variety of other warnings too (unreachable code is the only other one that comes to my mind... I'm somewhat sure there would be other cases).<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> This will be the “first” and there</div><div>may be templates that no one is using and this will break SDKs for those (something template writers may not be used to).</div><div><span class=""><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> Furthermore, if template is not used</div><div>in practice, there is no point issuing this diagnostic.</div></div></blockquote><div><br>Same would be true of non-template classes in headers, but we don't avoid those.<br></div></div></div></div></div></blockquote><div><br></div></span>Templates are special in that for whatever reasons, we skip diagnostics on them.</div><div><br></div><div><span class=""><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> Can you point to similar situations where templates are iterated over checking for method</div><div>inconsistencies or other types of diagnostics?</div></div></blockquote><div><br>Not that I recall/off-hand.<br></div></div></div></div></div></blockquote><div><br></div></span>If we do none. This will be first and people may have strong opinion on the “firsts” :).</div><div><span class=""><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br><br>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))<br></div></div></div></div></div></blockquote><div><br></div></span>I think similar situation is applicable to class templates which may not be instantiated. I just don’t know enough about usages etc. to make a strong argument one way</div><div>or the other. So, other opinions are most welcome.</div></div></blockquote><div><br>Yeah, I'd love to have some other voices here too<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>- Fariborz</div><span class=""><div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>- Fariborz</div><span><div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>- Fariborz</div><span><div><br><blockquote type="cite"><div><div dir="ltr"><br>- David</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 25, 2015 at 10:39 AM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div><div style="margin:0px;font-size:11px;font-family:Menlo">This patch restricts issuing -Winconsistent-missing-override when dealing with</div><div style="margin:0px;font-size:11px;font-family:Menlo">class template with dependent bases and dependent methods.</div><div style="margin:0px;font-size:11px;font-family:Menlo">Fixed pr22582 <a>rdar://19917107</a>.</div></div><div style="margin:0px;font-size:11px;font-family:Menlo">Please review.</div><div style="margin:0px;font-size:11px;font-family:Menlo"><br></div><div style="margin:0px;font-size:11px;font-family:Menlo">- Fariborz</div><div style="margin:0px;font-size:11px;font-family:Menlo"><br></div><div style="margin:0px;font-size:11px;font-family:Menlo"><span style="white-space:pre-wrap"> </span></div></div><br><div style="word-wrap:break-word"><div style="margin:0px;font-size:11px;font-family:Menlo"></div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</div></blockquote></div><br></span></div></blockquote></div><br></div></div>
</div></blockquote></div><br></span></div></blockquote></div><br></div></div>
</div></blockquote></div><br></span></div></blockquote></div><br></div></div>