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

[Dmitri Gribenko]

================
Comment at: lib/Sema/SemaDecl.cpp:7144
@@ +7143,3 @@
+    // dllimport cannot be used in variable definitions.
+    if (VD && VD->hasInit()) {
+      Diag(VD->getLocation(), diag::err_attribute_dll_import_var_init);
----------------
I'm not sure this is enough to catch all definitions (a definition is not required to have an initializer).

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4752-4753
@@ +4751,4 @@
+static InheritableAttr* GetDLLAttrFromDecl(Decl* D) {
+  DLLImportAttr* ImpAttr = D->getAttr<DLLImportAttr>();
+  if (ImpAttr)
+    return ImpAttr;
----------------
Better:
if (DLLImportAttr *ImpAttr = D->getAttr<DLLImportAttr>())
  return ImpAttr;

================
Comment at: lib/Sema/SemaDecl.cpp:7142
@@ -7137,1 +7141,3 @@
     }
+  } else if (DLLImportAttr* DLLAttr = ThisDecl->getAttr<DLLImportAttr>()) {
+    // dllimport cannot be used in variable definitions.
----------------
Star goes next to identifier (here and a few occurrences below.)

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4761-4762
@@ +4760,4 @@
+
+static void HandleRecordMemberDLLAttr(Decl* D, Sema* S,
+                           Attr* ClassAttr, CXXRecordDecl* RD) {
+  bool IsClassExported = ClassAttr &&
----------------
Please align 'Decl' and 'Attr'.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4770-4771
@@ +4769,4 @@
+  if (ClassAttr && MemberAttr) {
+    S->Diag(MemberAttr->getLocation(),
+      diag::err_attribute_dll_class_member_not_allowed)
+      << (IsClassExported ? 0 : 1);
----------------
Please align 'diag::' to 'MemberAttr'

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4807
@@ +4806,3 @@
+  if (IsRecordExported) {
+    // "All base classes of an exportable class must be exportable."
+    for (CXXRecordDecl::base_class_iterator I = RD->bases_begin();
----------------
If this is a quote, please state the source (or drop quotes).  (A few occurrences of this below, too.)

================
Comment at: lib/Sema/TargetAttributesSema.cpp:217
@@ -184,1 +216,3 @@
+  // Note: dllimport cannot be used in variable definitions.
+  // This check is performed in FinalizeDeclaration:SemaDecl.cpp
 
----------------
It is better to refer to method as Sema::FinalizeDeclaration.


================
Comment at: lib/Sema/TargetAttributesSema.cpp:219-220
@@ -184,1 +218,4 @@
 
+  // Note: dllimport cannot be used in function definitions.
+  // This check is performed in ActOnStartOfFunctionDef:SemaDecl.cpp
+
----------------
Method reference.


================
Comment at: test/Sema/dllimport-dllexport.cpp:14
@@ +13,3 @@
+
+typedef int __attribute__((dllexport)) type6; // expected-warning{{'dllexport' attribute only applies to variables, functions and tag types}}
+
----------------
Is there a reason this is not an error?



http://llvm-reviews.chandlerc.com/D34