[LLVMdev] Feedback required on proper dllexport/import implementation

[Gao, Yunzhong]

Hi Nico,
Thank you for your feedback. I'll try to answer your questions below.

> What problems do you expect with another approach? I have a local
> prototype that removes dllimport/export as linkage and uses function
> attributes and an extension to globals. I decorate dllexported functions with
> the Used attribute and it seems to work fine. This allows such functions to
> have linkonce_odr and weak_odr linkage and not be discarded.

My primary concern is what needs to be added to preserve current
compiler behavior where necessary. All the other approaches appear
to involve adding flags or attribute to an existing linkage type, so any
existing compiler codes that deal with that particular linkage type will
potentially have different behavior. For example, if you replace the
DLLExportLinkage with ExternalLinkage+DLLExport_attribute, then
codes that check hasExternalLinkage() will affect DLLExportLinkage
symbols as well, which may or may not be the right thing. For example,
LTO::addDefinedSymbol() currently does not handle dllimport/dllexport
symbols, but if you change the semantics of ExternalLinkage to include
dllimport/dllexport, then you could potentially have changed LTO
(although, that is not necessarily a bad thing; you just need to check
those places to make sure the compiler behavior still makes sense after
your change). And some contributors who have not caught up with
your change may still submit patches with the old semantics in mind,
which seems prone to errors.

If you really like your approach, I wonder if it makes sense to rename
the existing linkage type to something different, e.g., OldLinkage to
NewLinkage, so that future patches using hasOldLinkage() will have
to check their codes to make sure they work with the new semantics.

> Hm, I don't think so, because dllimported inline functions must never be
> instantiated, or else you get duplicated symbols. They are only allowed to be
> expanded. At the moment I solve this by checking for definitions during
> codegen (this means they survived the inliner) and dropping their body.

I agree. Hmm that is exactly why I said that dllimport inline functions bear
some similarity to the existing available_externally linkage. Maybe some
miscommunication has happened? I will make a guess as to what you
were saying.

Do you mean that you are dropping the function definition during clang
codegen instead of llvm backend? If the function definition does not
appear in the IR file, then the inliner won't be able to inline-expand the
function at call site, which seems to defeat the purpose of having an
inline definition. I solve this by having a new linkage type which works
just like available_externally (only for inline expansion but not for
out-of-line instantiation).

If I guessed wrong, please tell more details about your approach.


> > /* test1.c */
> > inline __declspec(dllimport) void foo(); inline void foo(); inline
> > void foo() { } void foo_user() { foo(); }
> > /* end of file */
> 
> Generally, MSVC allows a dllimport definition if the first declaration is
> specified as inline. This test case should import foo() (or expand it inline).

These examples are meant to demonstrate the theoretical issues when
C99 inline is used with dllexport/dllimport. MSVC does not support C99, so I
am not sure MSVC behavior is relevant. It appears to me that MSVC will use
C++ inline semantics even when compiling C files. It appears that both you
and Reid are okay with the approach of issuing an error message whenever a
C99 inline function is used with dllimport. You also seem okay with generating
an out-of-line, dllexported, definition when a C99 inline function is used with
dllexport.

> I have one issue in mind where I wonder if it's beneficial to be less strict than
> MSVC. As stated before, MSVC requires the inline keyword together with
> dllimport. This extends to member functions and prevents out-of-line
> definitions with inline:
> 
>    struct X1 {
>      inline void foo();
>      void bar();
>    };
>    void X::foo() {} // OK
>    inline void X::bar() {} // Error
> 
> Same goes for class and function templates. I stumbled on this in libc++
> where internal class templates are externed.

I guess you meant
__declspec(dllimport) struct X1 {
  inline void foo();
  void bar();
};
And you meant to have this declaration in a C++ file instead of a C file.

I have no objection to adding support for this usage.

- Gao.