[PATCH] Allow dllimport/dllexport on inline functions and get the linkage right

[David Majnemer]

On Wed, May 14, 2014 at 11:17 PM, Reid Kleckner <rnk at google.com> wrote:

> On Wed, May 14, 2014 at 11:01 PM, David Majnemer <david.majnemer at gmail.com
> > wrote:
>
>> On Wed, May 14, 2014 at 10:12 PM, Reid Kleckner <rnk at google.com> wrote:
>>
>>> On Wed, May 14, 2014 at 8:58 PM, David Majnemer <
>>> david.majnemer at gmail.com> wrote:
>>>
>>>> I'm a bit concerned with moving the DLLImport/DLLExport related linkage
>>>> calculation into Sema out of CodeGen.  I have a feeling that this will
>>>> trigger warn_static_local_in_extern_inline when a static local variable
>>>> inside of a dllimport'd function.
>>>>
>>>> Considering that other things like VTables and RTTI don't rely on the
>>>> GVALinakge mechanism and will need the ability to "upgrade" some linkage to
>>>> a DLLImport/DLLExport friendly linkage, I think I'd rather see a
>>>> DLLImport/DLLExport handled via a function which takes an arbitrary
>>>> llvm::GlobalVariable linkage and a Decl and figures out what the final
>>>> linkage should be.
>>>>
>>>> This kind of upgrading is common inside of CodeGen,
>>>> GetAddrOfGlobalTemporary is a notable example of this.]
>>>>
>>>
>>> IMO the linkage at the AST level should reflect what the user wrote, and
>>> the user used dllexport plus inline, giving GVA_StrongODR.  This is useful
>>> information for other Clang-based tools.
>>>
>>
>> I'd agree with this if we modeled stuff like fapple-kext in there but we
>> don't. There are things that we simply can't model like WeakAttr, we don't
>> know if we will end up with WeakODR or WeakAny until we try to evaluate the
>> initializer.
>>
>
> I'll have to read up on WeakAttr tomorrow.
>
>
>>  I think CodeGen will always have to munge GlobalValue linkages for
>>> entities that don't exist in the user's program, for example, vtables and
>>> global reference temporaries.
>>>
>>
>> This route will require us to duplicate handling of dllexport/dllimport
>> in both AST and CodeGen, I'd rather it just see it live in CodeGen.
>>
>
> True, we'll need both, but we already have this kind of code for entities
> that aren't part of the AST, like vtables.
>
>
>> I'd be happier with this if we can move stuff like -fapple-kext into
>> GVALinkage and we add more tests for static variables inside
>> dllimport/dllexport'd functions.
>>
>
> I view -fapple-kext as less of an AST-level, "what the user wrote" thing
> and more of a codegen option that hacks the ABI to maintain backwards
> compatibility with one of the Ancient Ones.
>

I don't think it's particularly useful for high-level examination of
linkage, it can flat out lie sometimes.  In the following case,
GVALinkageForVariable will claim that the VarDecl has GVA_DiscardableODR
when it in-fact will be emitted as "external global"

template <typename T>
struct S {
  static int i;
};

int *f() { return &S<void>::i; }

We should either commit to GVA as meaningful and GetGVALinkageFor* as
useful or we should consider them utility functions utilized by
Sema/CodeGen.

I tend to view them as the latter.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140514/120dc444/attachment.html>