avt77 marked an inline comment as done.
================
Comment at: lib/Sema/SemaDecl.cpp:5570
@@ -5565,3 +5569,3 @@
// and qualified friend declarations.
- // NB: MSVC converts such a declaration to dllexport.
+ // NB: MSVC converts such a declaration to dllexport that's why we do it
also.
bool IsInline = false, IsStaticDataMember = false, IsQualifiedFriend = false;
----------------
rsmith wrote:
> Just describe this as the semantics of this situation rather than suggesting
> this is some MSVC oddity we're emulating. "In such a case, the declaration is
> treated as if it were marked dllexport." or similar.
>
> It also seems bizarre for this behavior to apply for local extern
> declarations and qualified friend declarations. Does the "dllimport gets
> turned into dllexport" behavior actually only apply to the definition case?
> And does the definition need to be inline?
Yes, I see "dllimport gets turned into dllexport" for definitions only.
No, the definition should not be inline:
if (OldImportAttr && !HasNewAttr && **!IsInline**
================
Comment at: lib/Sema/SemaDecl.cpp:5595
@@ +5594,3 @@
+ // Replace DLLImportAttr with DLLExportAttr
+ OldDecl->dropAttr<DLLImportAttr>();
+ NewDecl->dropAttr<DLLImportAttr>();
----------------
rsmith wrote:
> Don't change attributes on a prior declaration; AST nodes should generally be
> immutable once created (this would lose source fidelity, and break under
> PCH/modules). Instead, make sure that anyone who looks at this gets the
> attribute from the appropriate (most recent) declaration and only change the
> attributes there.
I don't understand how I could "make sure that anyone...". Please, clarify.
================
Comment at: lib/Sema/SemaDecl.cpp:5596
@@ +5595,3 @@
+ OldDecl->dropAttr<DLLImportAttr>();
+ NewDecl->dropAttr<DLLImportAttr>();
+ OldDecl->addAttr(::new (S.Context) DLLExportAttr(
----------------
rsmith wrote:
> Is this really valid and treated as `__dllexport` if the new declaration
> explicitly specifies `__dllimport` (rather than inheriting it)?
The new declaration does not have explicitly specified __dllimport attribute:
if (OldImportAttr && **!HasNewAttr**. It's inherited.
================
Comment at: lib/Sema/SemaDecl.cpp:5600-5602
@@ +5599,5 @@
+ OldImportAttr->getSpellingListIndex()));
+ NewDecl->addAttr(::new (S.Context) DLLExportAttr(
+ NewImportAttr->getRange(), S.Context,
+ NewImportAttr->getSpellingListIndex()));
+ } else {
----------------
rsmith wrote:
> The new attribute should be marked implicit.
What do you mean? Please, clarify.
http://reviews.llvm.org/D18953
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
