[PATCH] Warn on zero-sized structures inside extern "C" block (PR5065)

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5721
@@ -5720,1 +5720,3 @@
   InGroup<CXXCompat>, DefaultIgnore;
+def warn_zero_size_struct_union : Warning<"%select{|empty }0"
+  "%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">,
----------------
Maybe name this `warn_..._union_in_extern_c` or similar?

================
Comment at: lib/Sema/SemaDecl.cpp:12074-12075
@@ +12073,4 @@
+      // For C++ filter out types that cannot be referenced in C code.
+      if (Record->getLexicalDeclContext()->isExternCContext() &&
+          !Record->isDependentType()) {
+        if (CXXRecordDecl *CXXRec = dyn_cast<CXXRecordDecl>(Record)) {
----------------
You could also check that `Record->getDeclContext()->getRedeclContext()->isFileContext()` here (that is, it's not a local class nor a member class).

================
Comment at: lib/Sema/SemaDecl.cpp:12076
@@ +12075,3 @@
+          !Record->isDependentType()) {
+        if (CXXRecordDecl *CXXRec = dyn_cast<CXXRecordDecl>(Record)) {
+          CheckForZeroSize =
----------------
This is redundant. In `CPlusPlus` mode, every `RecordDecl` is a `CXXRecordDecl`.

================
Comment at: lib/Sema/SemaDecl.cpp:12105
@@ -12082,1 +12104,3 @@
+              FieldType->isDependentType()  ||
+              FieldType->isUndeducedType()  ||
               !Context.getTypeSizeInChars(FieldType).isZero())
----------------
Non-static data members can't have deduced types, so this is impossible.

================
Comment at: lib/Sema/SemaDecl.cpp:12111-12132
@@ -12086,16 +12110,24 @@
 
-      // Empty structs are an extension in C (C99 6.7.2.1p7), but are allowed in
-      // C++.
-      if (ZeroSize)
-        Diag(RecLoc, diag::warn_zero_size_struct_union_compat) << IsEmpty
-            << Record->isUnion() << (NonBitFields > 1);
+      if (!getLangOpts().CPlusPlus) {
+        // Empty structs are an extension in C (C99 6.7.2.1p7), but are allowed
+        // in C++.
+        if (ZeroSize)
+          Diag(RecLoc, diag::warn_zero_size_struct_union_compat) << IsEmpty
+              << Record->isUnion() << (NonBitFields > 1);
 
-      // Structs without named members are extension in C (C99 6.7.2.1p7), but
-      // are accepted by GCC.
-      if (NonBitFields == 0) {
-        if (IsEmpty)
-          Diag(RecLoc, diag::ext_empty_struct_union) << Record->isUnion();
-        else
-          Diag(RecLoc, diag::ext_no_named_members_in_struct_union) << Record->isUnion();
+        // Structs without named members are extension in C (C99 6.7.2.1p7), but
+        // are accepted by GCC.
+        if (NonBitFields == 0) {
+          if (IsEmpty)
+            Diag(RecLoc, diag::ext_empty_struct_union) << Record->isUnion();
+          else
+            Diag(RecLoc, diag::ext_no_named_members_in_struct_union)
+              << Record->isUnion();
+        }
+      } else if (ZeroSize) {
+        // Empty structs are OK in C++ but will warn if its declaration is inside
+        // extern "C" block.
+        Diag(RecLoc, diag::warn_zero_size_struct_union) << IsEmpty
+          << Record->isUnion() << (NonBitFields > 1);
       }
     }
----------------
It would be simpler to organize this as:

  if (ZeroSize)
    Diag(..., getLangOpts().CPlusPlus ? diag::..._extern_c : diag::..._compat) ...

  if (NonBitFields == 0 && !getLangOpts().CPlusPlus) {
    ...
  }


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