rsmith added inline comments.
================
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;
----------------
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?
================
Comment at: lib/Sema/SemaDecl.cpp:5595
@@ +5594,3 @@
+ // Replace DLLImportAttr with DLLExportAttr
+ OldDecl->dropAttr<DLLImportAttr>();
+ NewDecl->dropAttr<DLLImportAttr>();
----------------
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.
================
Comment at: lib/Sema/SemaDecl.cpp:5596
@@ +5595,3 @@
+ OldDecl->dropAttr<DLLImportAttr>();
+ NewDecl->dropAttr<DLLImportAttr>();
+ OldDecl->addAttr(::new (S.Context) DLLExportAttr(
----------------
Is this really valid and treated as `__dllexport` if the new declaration
explicitly specifies `__dllimport` (rather than inheriting it)?
================
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 {
----------------
The new attribute should be marked implicit.
http://reviews.llvm.org/D18953
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
