[PATCH] D45244: Add missing vtable anchors

[Roman Lebedev via llvm-commits]

On Fri, Apr 20, 2018 at 9:25 PM, weiming zhao via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> There are tons of “weak vtable” issues found by enabling the -Wweak-vtables.
> I only fixed small part of them https://reviews.llvm.org/D45893 It’s very
> tedious :(
>
> Should we enable the warning and let code owners to address them?
Sadly, that is not really possible.
The buildbots use -Werror (as everyone everywhere always should!),
so that commit will immediately break the CI, and will be reverted.

One could enable it as not-a-error, but it will introduce unbearable
amounts of noise.
So i think one has to clean up all the instances of the warnings, and
then enable it.
Or, maybe it is possible to do it on per-directory basis.

Roman.

> On Apr 16, 2018, at 3:57 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Hmm - yeah, I'm not sure what the criteria would be for mixing rtti/no-rtti,
> etc.
>
> On Sun, Apr 15, 2018 at 8:45 PM weiming zhao <wmzhao.code at gmail.com> wrote:
>>
>> It’s very similar to the issue described in
>> http://lists.llvm.org/pipermail/llvm-dev/2016-February/095671.html
>> My code has RTTI and LLVM doesn’t. During static linking, it hits the
>> “undefined type info” error. After I put the anchors, the issues are solved.
>> My guess the weak vtable and the mixed RTTI/no-RTTI confused the linker
>> somehow. I don’t know the exact reason though.
>>
>>
>> On Apr 15, 2018, at 7:55 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Sat, Apr 14, 2018 at 10:33 PM Weiming Zhao via Phabricator
>> <reviews at reviews.llvm.org> wrote:
>>>
>>> weimingz added a comment.
>>>
>>> In https://reviews.llvm.org/D45244#1064905, @dblaikie wrote:
>>>
>>> > Great - thanks for sticking with it, sorry for my dodgy explanations!
>>> >
>>> > If you're adding in anchors, are you using -Wweak-vtables to find them?
>>> > If you're pushing through to make LLVM (& hopefully Clang/other LLVM
>>> > subprojects) -Wweak-vtables clean, perhaps you can turn on the warning in
>>> > the CMake config once it's clean so we don't regress this again? (it'd also
>>> > be interesting to do some kind of analysis to see whether avoiding weak
>>> > vtables is /actually/ worthwhile - like maybe making a
>>> > temporary/local/non-committed change to change all the explicit anchors into
>>> > inline functions (unanchoring them) & see if builds are much larger or
>>> > longer, etc)
>>>
>>>
>>> I got linker error when my code tries to statically link against some
>>> LLVM libs (like libLLVMOrc). It's a good idea to turn on -Wweak-vtables to
>>> check if we miss other cases. I will try it. Thanks
>>
>>
>> Hmm, I'm confused - you mean without these changes you were getting link
>> errors due to vtable symbols for types with weak vtables? I'm not sure why
>> that would be - your code should've been built with its own weak vtable, in
>> that situation.
>>
>> - Dave
>>
>>>
>>>
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> https://reviews.llvm.org/D45244
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>