[patch] Don't let virtual calls and dynamic casts call MarkVTableUsed()

[Reid Kleckner]

lgtm

On Tue, Jan 13, 2015 at 1:42 PM, Nico Weber <thakis at chromium.org> wrote:

> Tiny update: I missed a MarkVTableUsed() call in SemaInit.cpp. After
> removing that, Sema::BasePathInvolvesVirtualBase() is dead, so remove that
> too.
>
> On Tue, Jan 13, 2015 at 11:44 AM, Reid Kleckner <rnk at google.com> wrote:
>
>> On Tue, Jan 13, 2015 at 11:24 AM, Reid Kleckner <rnk at google.com> wrote:
>>
>>> On Tue, Jan 13, 2015 at 11:22 AM, Nico Weber <thakis at chromium.org>
>>> wrote:
>>>
>>>> On Tue, Jan 13, 2015 at 11:11 AM, Reid Kleckner <rnk at google.com> wrote:
>>>>
>>>>> I'm concerned that if you don't mark the vtable used enough, then
>>>>> codegen will crash trying to emit a vtable for a class that it assumed
>>>>> would be marked used because it's compiling a virtual call to a method of
>>>>> such a class. However, it looks like Rafael completely nuked the
>>>>> available_externally vtable emission optimization in r189852, which I
>>>>> forgot about. The vtable code still has lots of rigging to allow
>>>>> available_externally vtable emission, though.
>>>>>
>>>>
>>>> At the moment, the only thing in coding adding to the DeferredVTables
>>>> vector in codegen is getAddrOfVTable(), and that's only called for structor
>>>> body emission (and apple kext vcalls). So I think this should be fine.
>>>>
>>>
>>> Right, but we used to call it more, prior to PR13124 and r189852. I'm
>>> reading that bug to see if it's something we want to add back.
>>>
>>
>> I'm starting to think that even if we want to bring back the
>> available_externally vtable optimization, it should only happen
>> optimistically and should not rely on Sema to mark the vtable used. Looking
>> at PR13124, though, that seems difficult...
>>
>> Anyway, I drop my objections. The available_externally vtable
>> optimization doesn't exist and turns out to be very hard to implement in
>> Clang today. =/
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150114/ffc70dc1/attachment.html>