[PATCH] Add full semantic support for dllimport/export attributes

[Eli Friedman]

     if (!TDependent) {
-      OS << "      return A->clone(C);\n";
+      if (R.isSubClassOf(InhClass)) {
+        OS << "      InheritableAttr* CA = A->clone(C);\n";
+        OS << "      if (CA) CA->setInherited(A->isInherited());\n";
+        OS << "      return CA;\n";
+      } else {
+        OS << "      return A->clone(C);\n";
+      }
       OS << "    }\n";
       continue;
     }

If clone() isn't working, shouldn't you just fix it?  (If I'm
misunderstanding this change, please add a comment.)

@@ -259,9 +259,9 @@
       if (Triple.getOS() == llvm::Triple::Win32 ||
           Triple.getOS() == llvm::Triple::MinGW32) {
         switch (Attr.getKind()) {
-        case AttributeList::AT_DLLImport: HandleDLLImportAttr(D, Attr, S);
+        case AttributeList::AT_DLLImport: handleDLLImportAttr(D, Attr, S);
                                           return true;
-        case AttributeList::AT_DLLExport: HandleDLLExportAttr(D, Attr, S);
+        case AttributeList::AT_DLLExport: handleDLLExportAttr(D, Attr, S);
                                           return true;
         default:                          break;
         }

This doesn't look like it's actually a change.  If there's something
weird going on with whitespace, please commit separately.

In general, please go through your patch to remove whitespace-only changes.

+  // If we have a function-scope variable without explicit storage class, then
+  // give it external linkage.
+  VarDecl* VD = dyn_cast<VarDecl>(D);
+  if (!IsExported && VD && VD->getLexicalDeclContext()->isFunctionOrMethod() &&
+      VD->getStorageClass() == SC_None) {
+    VD->setStorageClass(SC_Extern);
+  }

setStorageClass changes the storage class *as written*.  That doesn't
seem appropriate here.

+  // Attribute cannot be applied to decls with non-external linkage.
+  if (!ND->isExternallyVisible()) {
+    bool IsExported = (Attr.getKind() == AttributeList::AT_DLLExport);
+    S.Diag(ND->getLocation(), diag::err_attribute_dllimpexp_not_extern_linkage)
+      << ND->getDeclName() << (IsExported ? 1 : 0);
+    ND->setInvalidDecl();
+    return false;
   }

Not sure isExternallyVisible() is the right predicate here; it doesn't
really make sense to apply DLLExport to e.g. a static local variable.
Maybe hasExternalFormalLinkage()?

This patch is a little overwhelming overall: there are a lot of subtle
changes.  Can you split out the non-RecordDecl-related changes into a
separate patch?

-Eli



On Fri, Jul 5, 2013 at 6:13 PM, Nico Rieck <nico.rieck at gmail.com> wrote:
> This is the first patch in a series to get full support for the dllexport/dllimport attributes and handles the semantic side.
>
> In particular it:
> - Adds support for class-wide export/import attributes (PR11170)
> - Handles dllexport/dllimport on inline functions and templates
> - Extends tests for globals and functions.
>
> It deviates from MSVC's behavior in the following cases:
> - If a dllimported global variable is redeclared without dllimport, the attribute is ignored with a warning. GCC does the same, MSVC makes the global dllexport.
> - Out-of-line member function definitions of dllimported class templates are allowed if the definition is declared as inline. MSVC allows this for classes, but not for class templates.
>
> In some cases a dllimport/dllexport attribute is ignored. If this happens on
> a member of an exported/imported class, we need a way to drop the attribute.
> This is implemented by propagating these attributes to every member during semantic analysis. To differentiate them from explicit member attributes, they have their inherited status set.
> To make this work a small change (or possibly fix?) to table-gen was required so that the inherited status of an attribute is preserved when instantiating template attributes.
>
>
> The semantics this patch should cover are:
>
> 1. dllexport and dllimport can be applied to functions, static data
>    members and global variables. Both can also be applied to typedefs
>    and enums (without effect) if -fms-extensions is specified, for all
>    other cases the attributes are ignored and a warning is issued. It is
>    an error to apply them to non-static data members.
>
> 2. dllexport requires a definition. The extern keyword can be used to
>    force a declaration.
>
> 3. dllimport requires a declaration.
>
> 4. dllexport exposes the function or variable with its decorated name.
>   a) For C++, this includes name mangling.
>   b) For C or extern "C" functions, this includes platform-specific
>      decoration for the calling convention.
>
> 5. dllexport takes precedence over dllimport. If both are specified,
>    dllimport is ignored and a warning is issued.
>
> 6. Classes
>   a) When applied to a class, dllexport or dllimport affect all static
>      and non-static member functions and all static data members,
>      including all non-deleted special members.
>   b) Everything that is accessible to the DLL's client according to C++
>      access rules is exported (including private data members referenced
>      in inline functions).
>   c) Members can be imported or exported selectively.
>   d) Explicit dllexport/dllimport attributes on members of exported/
>      imported classes are forbidden.
>   e) Base classes of an exported class should be exported. Emits a
>      warning otherwise. (-Wmissing-dllexport)
>   f) Virtual tables and typeinfo follow the emission rules of the
>      Itanium ABI, i.e. for an exported/imported dynamic class with key
>      function, the needed globals are exported/imported. If there is no
>      key function, they are emitted everywhere and are not exported/
>      imported.
>
> 7. Inline functions
>   a) dllexport can be applied to inline function definitions. The
>      function is always instantiated and exported, presumed to be
>      imported by another program.
>   b) dllimport can be applied to inline function definitions. Such
>      functions can be expanded, but never instantiated.
>   c) C99 inline functions cannot be exported or imported.
>
> 8. Templates
>   a) If a class template is exported, explicit and implicit
>      instantiations are also exported.
>   b) Non-exported/imported class templates can be exported/imported by
>      decorating an explicit instantiation.
>   c) Base classes that are template specializations must be instantiated
>      explicitly to export them.
>   d) If a class is marked as dllexport, any specializations of class
>      templates in the class hierarchy are implicitly marked as
>      dllexport. This means that class templates are explicitly
>      instantiated and the class's members must be defined.
>
>
> http://llvm-reviews.chandlerc.com/D1108
>
> Files:
>   include/clang/AST/DeclCXX.h
>   include/clang/Basic/DiagnosticGroups.td
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/AST/Decl.cpp
>   lib/Sema/SemaDecl.cpp
>   lib/Sema/SemaDeclCXX.cpp
>   lib/Sema/TargetAttributesSema.cpp
>   test/Rewriter/dllimport-typedef.c
>   test/Rewriter/missing-dllimport.c
>   test/Sema/dllexport.c
>   test/Sema/dllimport-dllexport.c
>   test/Sema/dllimport.c
>   test/SemaCXX/MicrosoftExtensions.cpp
>   test/SemaCXX/dllexport.cpp
>   test/SemaCXX/dllimport.cpp
>   utils/TableGen/ClangAttrEmitter.cpp
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>