> On Jul 17, 2015, at 10:34 AM, David Majnemer <david.majne...@gmail.com> wrote:
>
>
>
> On Fri, Jul 17, 2015 at 10:25 AM, Bob Wilson <bob.wil...@apple.com
> <mailto:bob.wil...@apple.com>> wrote:
>
>> On Jul 17, 2015, at 10:17 AM, Reid Kleckner <r...@google.com
>> <mailto:r...@google.com>> wrote:
>>
>> On Fri, Jul 17, 2015 at 4:45 AM, Aaron Ballman <aa...@aaronballman.com
>> <mailto:aa...@aaronballman.com>> wrote:
>> On Thu, Jul 16, 2015 at 8:42 PM, Bob Wilson <bob.wil...@apple.com
>> <mailto:bob.wil...@apple.com>> wrote:
>> > Clang used to silently ignore __declspec(novtable) for all platforms, but
>> > it is now implemented for Windows. However, we don’t check if the target
>> > is Windows. This does not work when using the Itanium ABI, where the class
>> > layout for complex class hierarchies is stored in the vtable. Leaving the
>> > vtable uninitialized on non-Windows platforms does not work in that case.
>> > It might be possible to honor the novtable attribute in some simple cases
>> > and either report an error or ignore it in more complex situations, but
>> > it’s not clear if that would be worthwhile. There is also value in having
>> > a simple and predictable behavior, so I am proposed the attached patch
>> > which simply ignores novtable on non-Windows platforms.
>>
>> I think it might actually be worth making it work. I have vague
>> recollections of Chromium developers wondering how to do the equivalent size
>> saving optimization on non-Windows targets. We'd have to pin down what makes
>> a "complex" class hierarchy. I'm assuming the fix would be to emit the vptr
>> store if the class has virtual bases.
>>
>> MSVC supports an Itanium build target. What does __declspec(novtable)
>> do there with the complex class layouts?
>>
>> I don't have Visual Studio installed with support for Itanium,
>> otherwise I would test this myself.
>>
>> I think Bob is talking about the Itanium C++ ABI, which I don't think MSVC
>> ever implemented. If they did, I wouldn't be surprised if they simply
>> ignored this declspec.
>
> Yes — I meant the Itanium C++ ABI.
>
> If someone else wants to sign up to implement this properly, I have no
> objections. In the meantime, my patch would fix the miscompiles that result
> from the current behavior. I’d still like to go ahead with it.
>
> My only qualm with the patch is that it wouldn't engage for MingW targets.
> It LGTM but the predicate needs tweaking to focus on the MSVC compatible
> targets..
That makes sense. The “TargetWindows” predicate is also used for the dllexport
and dllimport declspecs. Would it make sense to treat those in the same way? It
has been a while since I looked at MinGW.
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
