PATCH: don't mark pure derived method odr because of a qualified call to base method

[Nick Lewycky]

On 13 February 2013 16:38, John McCall <rjmccall at apple.com> wrote:

> On Feb 13, 2013, at 4:27 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Wed, Feb 13, 2013 at 4:16 PM, John McCall <rjmccall at apple.com> wrote:
>
>> On Feb 13, 2013, at 3:40 PM, Nick Lewycky <nlewycky at google.com> wrote:
>>
>> On 13 February 2013 10:53, John McCall <rjmccall at apple.com> wrote:
>>
>>> On Feb 13, 2013, at 12:53 AM, Nick Lewycky <nlewycky at google.com> wrote:
>>> > Thinking about this issue more, I'd like to retract this patch.
>>> Consider the case where we're implementing the rule that all ODR-used
>>> inline functions must have a definition. If Derived::fn is marked inline,
>>> we'll see a fictitious use of it and reject a valid translation unit.
>>> >
>>> > My next plan is to remove this code from Sema entirely and do the
>>> equivalent work inside codegen proper. No reason it can't keep track of
>>> probable virtual function targets when writing out a virtual dispatch.
>>>
>>> Ugh.  I am not thrilled by the idea of emitting massive amounts of code
>>> because something *might* devirtualize to it.
>>>
>>> Also, this potentially requires template instantiation.
>>>
>>
>> Me neither. However, Sema and CodeGen are already colluding to do this.
>> See SemaExpr.cpp:11168 (MarkExprReferenced), which may trigger
>> instantiation, and CGExprCXX.cpp:200 (EmitCXXMemberCallExpr) which assumes
>> the potential callee was already instantiated.
>>
>>
>> If we're willing to break some of the majestic beauty of the
>> frontend-isolated optimizer, we could queue up a late module pass which
>> 1) recognizes that we've emitted a reference to something that we're
>> required to define in every translation unit that references it (easy to do
>> with metadata),
>> 2) goes back to Sema and IR-gen to produce that code, and
>> 3) restarts the optimizer on the new functions.
>> Repeat until hopefully-rapid convergence.
>>
>
> That sounds like a path to optimization-dependent diagnostics, which we
> have previously gone to lengths to avoid. This case seems less severe,
> since it is likely that the diagnostics will be produced in *some*
> translation unit, but I still find it concerning.
>
>
> Do we really want to instantiate basically every virtual function in the
> translation unit even at -O0 just so that we can have a stable set of
> diagnostics?
>
> Do you have any estimates at all about how much extra code you'll be
> generating in an average translation unit?
>

This thread has run away. The only change I'm proposing right now is:

--- lib/Sema/SemaExpr.cpp       (revision 175024)
+++ lib/Sema/SemaExpr.cpp       (working copy)
@@ -11179,7 +11179,7 @@
   if (!MostDerivedClassDecl)
     return;
   CXXMethodDecl *DM =
MD->getCorrespondingMethodInClass(MostDerivedClassDecl);
-  if (!DM)
+  if (!DM || DM->isPure())
     return;
   SemaRef.MarkAnyDeclReferenced(Loc, DM, OdrUse);
 }

plus a testcase.
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130211/074046.html

Sorry I was unclear. I was never proposing we emit more (or less) code than
we do today, only that we stop reusing the odr bit on virtual functions to
mean something other than odr for virtual functions, which in turn requires
moving some logic out of sema and into codegen. I've dropped that idea,
since as you pointed out (and as I found out when I tried it) codegen'ing
the call may require instantiating things, such as noexcept expressions.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130213/7d2a3af9/attachment.html>