[cfe-commits] [PATCH] Improve handling of DLL import/export attributes

[Aaron Ballman]

On Mon, Sep 3, 2012 at 11:39 AM, João Matos
<reviews at llvm-reviews.chandlerc.com> wrote:
>
>   All fixed. Thanks a lot for the reviews so far. I will try to get Aaron to review the DLL stuff.

> Index: include/clang/AST/DeclCXX.h
> ===================================================================
> --- include/clang/AST/DeclCXX.h
> +++ include/clang/AST/DeclCXX.h
> @@ -779,6 +779,20 @@
>      return method_iterator(decls_end());
>    }
>
> +  /// Iterator access to static data members.  The method iterator visits
> +  /// all static data members of the class.
> +  typedef specific_decl_iterator<VarDecl> static_data_iterator;
> +
> +  /// static_data_begin - Static data member begin iterator.  Iterates in the
> +  // order the static data members were declared.
> +  static_data_iterator static_data_begin() const {
> +    return static_data_iterator(decls_begin());
> +  }
> +  /// static_data_end - Static data member end iterator.
> +  static_data_iterator static_data_end() const {
> +    return static_data_iterator(decls_end());
> +  }
> +

Is it really iterating only the static data?  From the looks of
things, this is iterating all VarDecl objects, regardless of storage
class.

> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1534,6 +1534,20 @@
>    "__attribute ((NSObject)) is for pointer types only">;
>  def err_attribute_can_be_applied_only_to_symbol_declaration : Error<
>    "%0 attribute can be applied only to symbol declaration">;
> +def err_attribute_dll_extern : Error<
> +  "%0 must have external linkage to be %select{exported|imported}1">;
> +def err_attribute_dll_import_var_init : Error<
> +  "imported variable cannot be defined">;
> +def err_attribute_dll_export_var_extern : Error<
> +  "attribute implies invalid external linkage in local scope">;
> +def warn_attribute_dll_export_base_class : Warning<
> +  "%0 is a base of an exported type and should be be exported">,
> +  InGroup<Microsoft>;
> +def warn_attribute_dll_export_accessible_type : Warning<
> +  "%0 is accessible from exported type and should be exported">,
> +  InGroup<Microsoft>;

It would be good to specify *what* type it is accessible from.  Also,
how would you "ignore" this warning if you wanted to (short of turning
it off entirely)?

> +def err_attribute_dll_class_member_not_allowed : Error<
> +  "member of %select{exported|imported}0 type may not declare DLL attributes">;
>  def err_attributes_are_not_compatible : Error<
>    "%0 and %1 attributes are not compatible">;
>  def err_attribute_wrong_number_arguments : Error<
> Index: lib/AST/Decl.cpp
> ===================================================================
> --- lib/AST/Decl.cpp
> +++ lib/AST/Decl.cpp
> @@ -1228,8 +1228,7 @@
>  }
>
>  VarDecl::DefinitionKind VarDecl::isThisDeclarationADefinition(
> -  ASTContext &C) const
> -{
> +  ASTContext &C) const {
>    // C++ [basic.def]p2:
>    //   A declaration is a definition unless [...] it contains the 'extern'
>    //   specifier or a linkage-specification and neither an initializer [...],
> @@ -1254,6 +1253,12 @@
>    //   initializer, the declaration is an external definition for the identifier
>    if (hasInit())
>      return Definition;
> +
> +  // A variable with the dllimport attribute implies a declaration.

Might want to provide some confirmation of this with an MSDN link to
the documentation.  Also, if this has been dllexported, then it is a
definition only which seems to be missing.

> +  if (hasAttr<DLLImportAttr>()) {
> +    return DeclarationOnly;
> +  }
> +
>    // AST for 'extern "C" int foo;' is annotated with 'extern'.
>    if (hasExternalStorage())
>      return DeclarationOnly;
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -7100,11 +7100,15 @@
>  Sema::FinalizeDeclaration(Decl *ThisDecl) {
>    // Note that we are no longer parsing the initializer for this declaration.
>    ParsingInitForAutoVars.erase(ThisDecl);
> +
> +  if (!ThisDecl)
> +    return;
> +
> +  VarDecl *VD = dyn_cast<VarDecl>(ThisDecl);

If you're going to hoist things from the if, why not go the whole way?

VarDecl *VD = dyn_cast_or_null<VarDecl>(ThisDecl);
if (!VD)
  return;

>    // Now we have parsed the initializer and can update the table of magic
>    // tag values.
> -  if (ThisDecl && ThisDecl->hasAttr<TypeTagForDatatypeAttr>()) {
> -    const VarDecl *VD = dyn_cast<VarDecl>(ThisDecl);
> +  if (ThisDecl->hasAttr<TypeTagForDatatypeAttr>()) {
>      if (VD && VD->getType()->isIntegralOrEnumerationType()) {
>        for (specific_attr_iterator<TypeTagForDatatypeAttr>
>                 I = ThisDecl->specific_attr_begin<TypeTagForDatatypeAttr>(),
> @@ -7135,6 +7139,13 @@
>                                     I->getMustBeNull());
>        }
>      }
> +  } else if (DLLImportAttr *DLLAttr = ThisDecl->getAttr<DLLImportAttr>()) {
> +    // dllimport cannot be used in variable definitions.
> +    if (VD && VD->hasInit()) {
> +      Diag(VD->getLocation(), diag::err_attribute_dll_import_var_init);
> +      VD->setInvalidDecl();
> +      return;
> +    }

You should probably also be checking the storage class specifier since
dllimport must have external linkage.  Eg)

static __declspec( dllimport ) int l;  // This is an error

(More info on this at http://msdn.microsoft.com/en-us/library/y4h7bcy6)

>    }
>  }
>
> Index: lib/Sema/SemaDeclCXX.cpp
> ===================================================================
> --- lib/Sema/SemaDeclCXX.cpp
> +++ lib/Sema/SemaDeclCXX.cpp
> @@ -4747,6 +4747,105 @@
>    }
>  }
>
> +// Returns a DLL attribute from the declaration.
> +static InheritableAttr *GetDLLAttrFromDecl(Decl *D) {
> +  if (DLLImportAttr *ImpAttr = D->getAttr<DLLImportAttr>())
> +    return ImpAttr;
> +  if (DLLExportAttr *ExpAttr = D->getAttr<DLLExportAttr>())
> +    return ExpAttr;
> +  return NULL;
> +}

Why is dllimport preferred over dllexport?  This is backwards from
what MSDN documents the behavior to be
(http://msdn.microsoft.com/en-us/library/twa2aw10).  Also, the second
if could be an else if to be more explicit that it can only be one or
the other.

> +
> +static void HandleRecordMemberDLLAttr(Decl *D, Sema *S,
> +                                      Attr *ClassAttr, CXXRecordDecl *RD) {
> +  bool IsClassExported = ClassAttr && ClassAttr->getKind() == attr::DLLExport;
> +
> +  InheritableAttr *MemberAttr = GetDLLAttrFromDecl(D);
> +
> +  // If the class has a DLL attribute, the member cannot declare one explicitly.
> +  if (ClassAttr && MemberAttr) {
> +    S->Diag(MemberAttr->getLocation(),
> +            diag::err_attribute_dll_class_member_not_allowed)
> +              << (IsClassExported ? 0 : 1);

Might want a "previously declared here" note to point out that this
conflict is happening because of the class.

> +    return;
> +  }
> +
> +  // FIXME: Check for accessibility.

When will this accessibility check be implemented?  Since this is all
new code, I'd rather not add the FIXME unless this is a major
undertaking worthy of a separate patch.

> +  if (ClassAttr) {
> +    InheritableAttr *NewAttr =
> +        cast<InheritableAttr>(ClassAttr->clone(S->getASTContext()));
> +    NewAttr->setInherited(true);
> +    D->addAttr(NewAttr);
> +  }

This if could be hoisted into the previous if statement, could it not?

> +
> +  // Warn if type to check is a non-exported class.
> +  bool IsMemberExported = MemberAttr &&
> +      MemberAttr->getKind() == attr::DLLExport;
> +
> +  if (IsClassExported || IsMemberExported) {

This if statement doesn't match with the previous comment about
warning if it's a non-exported class.

> +    if (RD && !RD->hasAttr<DLLExportAttr>()) {
> +      S->Diag(D->getLocation(),
> +              diag::warn_attribute_dll_export_accessible_type)
> +                << RD->getDeclName();

May want a "previously declared here" note to point out why this
declaration is accessible

> +    }
> +  }
> +}
> +
> +// Performs DLL attribute semantic checks on the declaration.
> +// The semantics of these are explained in MSDN:
> +//   http://msdn.microsoft.com/en-us/library/81h27t8c(v=vs.110).aspx.
> +static void HandleDLLAttrs(CXXRecordDecl *RD, Sema *S) {
> +  if (!RD)
> +    return;
> +
> +  InheritableAttr *RecordAttr = GetDLLAttrFromDecl(RD);
> +  bool IsRecordExported = RecordAttr &&
> +      RecordAttr->getKind() == attr::DLLExport;
> +
> +  if (IsRecordExported) {
> +    // "All base classes of an exportable class must be exportable."
> +    for (CXXRecordDecl::base_class_iterator I = RD->bases_begin();
> +        I != RD->bases_end(); ++I) {
> +      CXXBaseSpecifier *BS = I;
> +      const CXXRecordDecl *BD = BS->getType()->getAsCXXRecordDecl();
> +
> +      if (BD && !BD->hasAttr<DLLExportAttr>()) {
> +        S->Diag(BS->getLocStart(),
> +                diag::warn_attribute_dll_export_base_class)
> +                  << BD->getDeclName();

This diagnostic seems like it will appear in the wrong location.

class A {};
class __declspec( dllexport ) B : public A {};
                                                           ^
So I think the diagnostic should point to the current record's
dllexport and then have a note pointing to the base class declaration
tying the two together.  Also, I wonder if we could add a fixit to
supply the dllexport for the definition of the base class?

> +      }
> +    }
> +  }
> +
> +  // "As a rule, everything that is accessible to the DLL's client (according
> +  // to C++ access rules) should be part of the exportable interface.
> +  // This includes private data members referenced in inline functions."
> +  // FIXME: Add more checks for those complex cases.

Why not add those checks right now?  Or at the very least, make the
FIXME something actionable.  What complex cases still need covering?

> +
> +  for (CXXRecordDecl::method_iterator I = RD->method_begin();
> +      I != RD->method_end(); ++I) {
> +    CXXMethodDecl *MD = *I;
> +    CXXRecordDecl *RD = MD->getResultType()->getAsCXXRecordDecl();
> +    HandleRecordMemberDLLAttr(MD, S, RecordAttr, RD);
> +  }
> +
> +  // "Moreover, all accessible members that are also classes must be exportable."
> +  for (CXXRecordDecl::field_iterator I = RD->field_begin();
> +      I != RD->field_end(); ++I) {
> +    FieldDecl *FD = *I;
> +    CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
> +    HandleRecordMemberDLLAttr(FD, S, RecordAttr, RD);
> +  }
> +
> +  // Handle static data members.
> +  for (CXXRecordDecl::static_data_iterator I = RD->static_data_begin();
> +      I != RD->static_data_end(); ++I) {
> +    VarDecl *VD = *I;
> +    CXXRecordDecl *RD = VD->getType()->getAsCXXRecordDecl();
> +    HandleRecordMemberDLLAttr(VD, S, RecordAttr, RD);
> +  }
> +}
> +
>  void Sema::ActOnFinishCXXMemberSpecification(Scope* S, SourceLocation RLoc,
>                                               Decl *TagDecl,
>                                               SourceLocation LBrac,
> @@ -4765,6 +4864,8 @@
>        l->getName();
>    }
>
> +  HandleDLLAttrs(dyn_cast<CXXRecordDecl>(TagDecl), this);
> +
>    ActOnFields(S, RLoc, TagDecl, llvm::makeArrayRef(
>                // strict aliasing violation!
>                reinterpret_cast<Decl**>(FieldCollector->getCurFields()),
> Index: lib/Sema/TargetAttributesSema.cpp
> ===================================================================
> --- lib/Sema/TargetAttributesSema.cpp
> +++ lib/Sema/TargetAttributesSema.cpp
> @@ -151,6 +151,52 @@
>                                                             S.Context));
>  }
>
> +static bool HandleDLLShared(Decl *D, const AttributeList &Attr, Sema &S) {
> +  // Check the attribute arguments.
> +  if (Attr.getNumArgs() != 0) {
> +    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
> +    return false;
> +  }
> +
> +  // Attribute can be applied only to functions, variables or tag types.
> +  if (!(isa<VarDecl>(D) || isa<FunctionDecl>(D) || isa<TagDecl>(D))) {
> +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> +      << Attr.getName() << 13 /* variables, functions and tag types */;
> +    return false;
> +  }
> +
> +  bool isDLLExport = (Attr.getKind() == AttributeList::AT_DLLExport);
> +
> +  // If we have a function-scope variable that does not have an explicit
> +  // storage class, then make sure it will end up with external linkage.
> +  VarDecl* VD = dyn_cast<VarDecl>(D);
> +  if (VD && VD->getLexicalDeclContext()->isFunctionOrMethod()
> +      && VD->getStorageClass() == SC_None) {
> +    if (isDLLExport) {
> +      S.Diag(Attr.getLoc(), diag::err_attribute_dll_export_var_extern);
> +      VD->setInvalidDecl();
> +      return false;

I wonder if this is a case for a fixit, since we could conceivably
just add extern to the declaration and continue.

> +    }
> +    VD->setStorageClass(SC_Extern);
> +  }
> +
> +  // DLL attributes cannot be applied to non-external linkage entities.
> +  NamedDecl* ND = dyn_cast<NamedDecl>(D);
> +
> +  if (!ND)
> +    return false;
> +
> +  if(!isExternalLinkage(ND->getLinkage())) {

If this was a variable declaration, didn't you just set the linkage by
setting the storage class on VD?

> +    int Opt = isDLLExport ? 0 /*exported*/ : 1 /*imported*/;
> +    S.Diag(VD->getLocation(), diag::err_attribute_dll_extern)
> +      << ND->getDeclName() << Opt;
> +    ND->setInvalidDecl();
> +    return false;
> +  }
> +
> +  return true;
> +}
> +
>  DLLImportAttr *Sema::mergeDLLImportAttr(Decl *D, SourceRange Range) {
>    if (D->hasAttr<DLLExportAttr>()) {
>      Diag(Range.getBegin(), diag::warn_attribute_ignored) << "dllimport";
> @@ -164,26 +210,18 @@
>  }
>
>  static void HandleDLLImportAttr(Decl *D, const AttributeList &Attr, Sema &S) {
> -  // check the attribute arguments.
> -  if (Attr.getNumArgs() != 0) {
> -    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
> +  if (!HandleDLLShared(D, Attr, S))
>      return;
> -  }
>
> -  // Attribute can be applied only to functions or variables.
> -  FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
> -  if (!FD && !isa<VarDecl>(D)) {
> -    // Apparently Visual C++ thinks it is okay to not emit a warning
> -    // in this case, so only emit a warning when -fms-extensions is not
> -    // specified.

That's because you can dllimport a class (at least you can according
to http://msdn.microsoft.com/en-us/library/81h27t8c).  I don't think
this is a valid diagnostic.

> -    if (!S.getLangOpts().MicrosoftExt)
> -      S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> -        << Attr.getName() << 2 /*variable and function*/;
> -    return;
> -  }
> +  // Note: dllimport cannot be used in variable definitions.
> +  // This check is performed in Sema::FinalizeDeclaration().
>
> +  // Note: dllimport cannot be used in function definitions.
> +  // This check is performed in Sema::ActOnStartOfFunctionDef().
> +
>    // Currently, the dllimport attribute is ignored for inlined functions.
>    // Warning is emitted.
> +  FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
>    if (FD && FD->isInlineSpecified()) {
>      S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << "dllimport";
>      return;

I don't think this is valid either as it is explicitly allowed by MSVC
(even though I think it's a bit strange):

"You can also define as inline a function declared with the dllimport
attribute. In this case, the function can be expanded (subject to /Ob
specifications), but never instantiated. In particular, if the address
of an inline imported function is taken, the address of the function
residing in the DLL is returned. This behavior is the same as taking
the address of a non-inline imported function."

Also, you are missing a check to ensure dllimport hasn't already been
specified.  Currently you can do __declspec( dllimport ) __declspec(
dllimport ) int x;  and it will not warn.

> @@ -207,22 +245,13 @@
>  }
>
>  static void HandleDLLExportAttr(Decl *D, const AttributeList &Attr, Sema &S) {
> -  // check the attribute arguments.
> -  if (Attr.getNumArgs() != 0) {
> -    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
> +  if (!HandleDLLShared(D, Attr, S))
>      return;
> -  }
>
> -  // Attribute can be applied only to functions or variables.
> -  FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
> -  if (!FD && !isa<VarDecl>(D)) {

Again, I don't think this is correct since you can dllexport a class.

> -    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> -      << Attr.getName() << 2 /*variable and function*/;
> -    return;
> -  }
> -
>    // Currently, the dllexport attribute is ignored for inlined functions, unless
>    // the -fkeep-inline-functions flag has been used. Warning is emitted;
> +  // FIXME: MSDN says this should work for inline functions in all cases.

Are you intending to address this FIXME?

> +  FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
>    if (FD && FD->isInlineSpecified()) {
>      // FIXME: ... unless the -fkeep-inline-functions flag has been used.

Or this one?

>      S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << "dllexport";

Also, you are missing a check to ensure dllexport hasn't already been
specified.   This can likely go into the shared check.

> @@ -240,6 +269,7 @@
>      X86AttributesSema() { }
>      bool ProcessDeclAttribute(Scope *scope, Decl *D,
>                                const AttributeList &Attr, Sema &S) const {
> +      // FIXME: The DLL interface attributes are not x86-specific.
>        const llvm::Triple &Triple(S.Context.getTargetInfo().getTriple());
>        if (Triple.getOS() == llvm::Triple::Win32 ||
>            Triple.getOS() == llvm::Triple::MinGW32) {
> Index: test/Rewriter/dllimport-typedef.c
> ===================================================================
> --- test/Rewriter/dllimport-typedef.c
> +++ test/Rewriter/dllimport-typedef.c
> @@ -11,7 +11,7 @@
>
>  // CHECK-NEG: error: void function 'bar' should not return a value
>  // CHECK-NEG: 1 error generated
> -// CHECK-POS: warning: 'dllimport' attribute only applies to variables and functions
> +// CHECK-POS: warning: 'dllimport' attribute only applies to variables, functions and tag types
>  // CHECK-POS: error: void function 'bar' should not return a value
>  // CHECK-POS: 1 warning and 1 error generated
>
> Index: test/Sema/dllimport-dllexport.c
> ===================================================================
> --- test/Sema/dllimport-dllexport.c
> +++ test/Sema/dllimport-dllexport.c
> @@ -1,43 +0,0 @@
> -// RUN: %clang_cc1 -triple i386-mingw32 -fsyntax-only -verify %s
> -// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -verify %s
> -
> -inline void __attribute__((dllexport)) foo1(){} // expected-warning{{dllexport attribute ignored}}
> -inline void __attribute__((dllimport)) foo2(){} // expected-warning{{dllimport attribute ignored}}
> -
> -void __attribute__((dllimport)) foo3(){} // expected-error{{dllimport attribute can be applied only to symbol declaration}}
> -
> -void __attribute__((dllimport, dllexport)) foo4(); // expected-warning{{dllimport attribute ignored}}
> -
> -void __attribute__((dllexport)) foo5();
> -void __attribute__((dllimport)) foo5(); // expected-warning{{dllimport attribute ignored}}
> -
> -typedef int __attribute__((dllexport)) type6; // expected-warning{{'dllexport' attribute only applies to variables and functions}}
> -
> -typedef int __attribute__((dllimport)) type7; // expected-warning{{'dllimport' attribute only applies to variables and functions}}
> -
> -void __attribute__((dllimport)) foo6();
> -void foo6(){} // expected-warning {{'foo6' redeclared without dllimport attribute: previous dllimport ignored}}
> -
> -// PR6269
> -inline void __declspec(dllexport) foo7(){} // expected-warning{{dllexport attribute ignored}}
> -inline void __declspec(dllimport) foo8(){} // expected-warning{{dllimport attribute ignored}}
> -
> -void __declspec(dllimport) foo9(){} // expected-error{{dllimport attribute can be applied only to symbol declaration}}
> -
> -void __declspec(dllimport) __declspec(dllexport) foo10(); // expected-warning{{dllimport attribute ignored}}
> -
> -void __declspec(dllexport) foo11();
> -void __declspec(dllimport) foo11(); // expected-warning{{dllimport attribute ignored}}
> -
> -typedef int __declspec(dllexport) type1; // expected-warning{{'dllexport' attribute only applies to variables and functions}}
> -
> -typedef int __declspec(dllimport) type2; // expected-warning{{'dllimport' attribute only applies to variables and functions}}
> -
> -void __declspec(dllimport) foo12();
> -void foo12(){} // expected-warning {{'foo12' redeclared without dllimport attribute: previous dllimport ignored}}
> -
> -void __attribute__((dllimport)) foo13(); // expected-warning{{dllimport attribute ignored}}
> -void __attribute__((dllexport)) foo13();
> -
> -extern int foo14 __attribute__((dllexport));
> -extern int foo14 __attribute__((dllimport));  // expected-warning{{dllimport attribute ignored}}

Why are you getting rid of this entire file?  If you have C++-specific
tests to add for things like classes, then you should add a C++ file
and put those tests there instead of getting rid of the entire C file.
 Also, it might be worth testing export/import structures from C.

> Index: test/Sema/dllimport-dllexport.cpp
> ===================================================================
> --- test/Sema/dllimport-dllexport.cpp
> +++ test/Sema/dllimport-dllexport.cpp
> @@ -0,0 +1,75 @@
> +// RUN: %clang_cc1 -triple i386-mingw32 -fsyntax-only -verify %s
> +// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -verify %s
> +
> +inline void __attribute__((dllexport)) foo1(){} // expected-warning{{dllexport attribute ignored}}
> +inline void __attribute__((dllimport)) foo2(){} // expected-warning{{dllimport attribute ignored}}
> +
> +void __attribute__((dllimport)) foo3(){} // expected-error{{dllimport attribute can be applied only to symbol declaration}}
> +
> +void __attribute__((dllimport, dllexport)) foo4(); // expected-warning{{dllimport attribute ignored}}
> +
> +void __attribute__((dllexport)) foo5();
> +void __attribute__((dllimport)) foo5(); // expected-warning{{dllimport attribute ignored}}
> +
> +// This should really be an error, but since MSVC does even generate
> +// a diagnostic in this case, we only generate a warning to not break
> +// compatibility with existing code.
> +
> +typedef int __attribute__((dllexport)) type6; // expected-warning{{'dllexport' attribute only applies to variables, functions and tag types}}
> +
> +typedef int __attribute__((dllimport)) type7; // expected-warning{{'dllimport' attribute only applies to variables, functions and tag types}}
> +
> +void __attribute__((dllimport)) foo6();
> +void foo6(){} // expected-warning {{'foo6' redeclared without dllimport attribute: previous dllimport ignored}}
> +
> +// PR6269
> +inline void __declspec(dllexport) foo7(){} // expected-warning{{dllexport attribute ignored}}
> +inline void __declspec(dllimport) foo8(){} // expected-warning{{dllimport attribute ignored}}
> +
> +void __declspec(dllimport) foo9(){} // expected-error{{dllimport attribute can be applied only to symbol declaration}}
> +
> +void __declspec(dllimport) __declspec(dllexport) foo10(); // expected-warning{{dllimport attribute ignored}}
> +
> +void __declspec(dllexport) foo11();
> +void __declspec(dllimport) foo11(); // expected-warning{{dllimport attribute ignored}}
> +
> +typedef int __declspec(dllexport) type1; // expected-warning{{'dllexport' attribute only applies to variables, functions and tag types}}
> +
> +typedef int __declspec(dllimport) type2; // expected-warning{{'dllimport' attribute only applies to variables, functions and tag types}}
> +
> +void __declspec(dllimport) foo12();
> +void foo12(){} // expected-warning {{'foo12' redeclared without dllimport attribute: previous dllimport ignored}}
> +
> +void __attribute__((dllimport)) foo13(); // expected-warning{{dllimport attribute ignored}}
> +void __attribute__((dllexport)) foo13();
> +
> +extern int foo14 __attribute__((dllexport));
> +extern int foo14 __attribute__((dllimport));  // expected-warning{{dllimport attribute ignored}}
> +
> +static int foo15 __declspec(dllexport); // expected-error{{'foo15' must have external linkage to be exported}}
> +
> +int __declspec(dllimport) bar0 = 2; // expected-error {{imported variable cannot be defined}}
> +
> +int __declspec(dllimport) bar1; // expected-warning {{dllimport attribute ignored}}
> +int __declspec(dllexport) bar1;
> +
> +extern int __declspec(dllexport) bar2;
> +int __declspec(dllimport) bar2; // expected-warning {{dllimport attribute ignored}}
> +
> +int foof() { int foofi __declspec(dllexport); } // expected-error {{attribute implies invalid external linkage in local scope}}
> +
> +class __declspec(dllexport) fooc {};
> +
> +class __declspec(dllimport) bar2 {
> +  __declspec(dllimport) int foo(); // expected-error {{member of imported type may not declare DLL attributes}}
> +};
> +
> +class A {};
> +class __declspec(dllexport) B { A a; }; // expected-warning {{'A' is accessible from exported type and should be exported}}
> +
> +class __declspec(dllexport) C : A {}; // expected-warning {{'A' is a base of an exported type and should be be exported}}
> +
> +
> +
> +
> +

There's a lot of extra whitespace here that can be removed.

I'd like to see more test cases as well, such as ones missing from
MSDN, as well as edge cases like duplicate dllimport or dllexport
attributes, etc

Thanks!

~Aaron