From 142ab06ffb2ec286917554aa5d945323a1ebf359 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Thu, 14 Nov 2013 02:13:03 +0000 Subject: [PATCH] Added warning on structures/unions that are empty or contain only bit fields of zero size. Warnings are generated in C++ mode and if only such type is defined inside extern "C" block. The patch fixed PR5065. Differential Revision: http://llvm-reviews.chandlerc.com/D2151 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@194653 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclBase.h | 8 ++++ include/clang/Basic/DiagnosticGroups.td | 4 +- include/clang/Basic/DiagnosticSemaKinds.td | 3 ++ lib/AST/Decl.cpp | 23 ++-------- lib/AST/DeclBase.cpp | 18 ++++++++ lib/Sema/SemaDecl.cpp | 42 +++++++++++++------ .../basic.lookup/basic.lookup.argdep/p2.cpp | 2 +- test/SemaCXX/extern-c.cpp | 19 ++++++++- test/SemaCXX/storage-class.cpp | 2 +- 9 files changed, 85 insertions(+), 36 deletions(-) diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index f8c69f9929..26eea64f9d 100644 --- a/include/clang/AST/DeclBase.h +++ b/include/clang/AST/DeclBase.h @@ -1159,6 +1159,14 @@ public: /// C++0x scoped enums), and C++ linkage specifications. bool isTransparentContext() const; + /// \brief Determines whether this context or some of its ancestors is a + /// linkage specification context that specifies C linkage. + bool isExternCContext() const; + + /// \brief Determines whether this context or some of its ancestors is a + /// linkage specification context that specifies C++ linkage. + bool isExternCXXContext() const; + /// \brief Determine whether this declaration context is equivalent /// to the declaration context DC. bool Equals(const DeclContext *DC) const { diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index be9ea674d0..a590ae8ea6 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -49,6 +49,7 @@ def BuiltinMacroRedefined : DiagGroup<"builtin-macro-redefined">; def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">; def C99Compat : DiagGroup<"c99-compat">; def CXXCompat: DiagGroup<"c++-compat">; +def ExternCCompat : DiagGroup<"extern-c-compat">; def GNUCaseRange : DiagGroup<"gnu-case-range">; def CastAlign : DiagGroup<"cast-align">; def : DiagGroup<"cast-qual">; @@ -494,7 +495,8 @@ def Most : DiagGroup<"most", [ ObjCMissingSuperCalls, OverloadedVirtual, PrivateExtern, - SelTypeCast + SelTypeCast, + ExternCCompat ]>; // Thread Safety warnings diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index aae9df7ac4..d9b0a01ea5 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5728,6 +5728,9 @@ def ext_no_named_members_in_struct_union : Extension< def warn_zero_size_struct_union_compat : Warning<"%select{|empty }0" "%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">, InGroup, DefaultIgnore; +def warn_zero_size_struct_union_in_extern_c : Warning<"%select{|empty }0" + "%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">, + InGroup; } // End of general sema category. // inline asm. diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index e81444ebe7..fe6f5fa5f4 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1698,27 +1698,12 @@ bool VarDecl::isExternC() const { return isExternCTemplate(*this); } -static bool isLinkageSpecContext(const DeclContext *DC, - LinkageSpecDecl::LanguageIDs ID) { - while (DC->getDeclKind() != Decl::TranslationUnit) { - if (DC->getDeclKind() == Decl::LinkageSpec) - return cast(DC)->getLanguage() == ID; - DC = DC->getParent(); - } - return false; -} - -template -static bool isInLanguageSpecContext(T *D, LinkageSpecDecl::LanguageIDs ID) { - return isLinkageSpecContext(D->getLexicalDeclContext(), ID); -} - bool VarDecl::isInExternCContext() const { - return isInLanguageSpecContext(this, LinkageSpecDecl::lang_c); + return getLexicalDeclContext()->isExternCContext(); } bool VarDecl::isInExternCXXContext() const { - return isInLanguageSpecContext(this, LinkageSpecDecl::lang_cxx); + return getLexicalDeclContext()->isExternCXXContext(); } VarDecl *VarDecl::getCanonicalDecl() { return getFirstDecl(); } @@ -2407,11 +2392,11 @@ bool FunctionDecl::isExternC() const { } bool FunctionDecl::isInExternCContext() const { - return isInLanguageSpecContext(this, LinkageSpecDecl::lang_c); + return getLexicalDeclContext()->isExternCContext(); } bool FunctionDecl::isInExternCXXContext() const { - return isInLanguageSpecContext(this, LinkageSpecDecl::lang_cxx); + return getLexicalDeclContext()->isExternCXXContext(); } bool FunctionDecl::isGlobal() const { diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index be1992a4d5..121c5a671a 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -806,6 +806,24 @@ bool DeclContext::isTransparentContext() const { return false; } +static bool isLinkageSpecContext(const DeclContext *DC, + LinkageSpecDecl::LanguageIDs ID) { + while (DC->getDeclKind() != Decl::TranslationUnit) { + if (DC->getDeclKind() == Decl::LinkageSpec) + return cast(DC)->getLanguage() == ID; + DC = DC->getParent(); + } + return false; +} + +bool DeclContext::isExternCContext() const { + return isLinkageSpecContext(this, clang::LinkageSpecDecl::lang_c); +} + +bool DeclContext::isExternCXXContext() const { + return isLinkageSpecContext(this, clang::LinkageSpecDecl::lang_cxx); +} + bool DeclContext::Encloses(const DeclContext *DC) const { if (getPrimaryContext() != this) return getPrimaryContext()->Encloses(DC); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index a7b37c55e9..c0be341b66 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -12085,8 +12085,21 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl, if (Record->hasAttrs()) CheckAlignasUnderalignment(Record); - // Check if the structure/union declaration is a language extension. + // Check if the structure/union declaration is a type that can have zero + // size in C. For C this is a language extension, for C++ it may cause + // compatibility problems. + bool CheckForZeroSize; if (!getLangOpts().CPlusPlus) { + CheckForZeroSize = true; + } else { + // For C++ filter out types that cannot be referenced in C code. + CXXRecordDecl *CXXRecord = cast(Record); + CheckForZeroSize = + CXXRecord->getLexicalDeclContext()->isExternCContext() && + !CXXRecord->isDependentType() && + CXXRecord->isCLike(); + } + if (CheckForZeroSize) { bool ZeroSize = true; bool IsEmpty = true; unsigned NonBitFields = 0; @@ -12106,19 +12119,22 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl, } } - // 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); + // Empty structs are an extension in C (C99 6.7.2.1p7). They are + // allowed in C++, but warn if its declaration is inside + // extern "C" block. + if (ZeroSize) { + Diag(RecLoc, getLangOpts().CPlusPlus ? + diag::warn_zero_size_struct_union_in_extern_c : + 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 && !getLangOpts().CPlusPlus) { + Diag(RecLoc, IsEmpty ? diag::ext_empty_struct_union : + diag::ext_no_named_members_in_struct_union) + << Record->isUnion(); } } } else { diff --git a/test/CXX/basic/basic.lookup/basic.lookup.argdep/p2.cpp b/test/CXX/basic/basic.lookup/basic.lookup.argdep/p2.cpp index e61df3e586..e352bbe83c 100644 --- a/test/CXX/basic/basic.lookup/basic.lookup.argdep/p2.cpp +++ b/test/CXX/basic/basic.lookup/basic.lookup.argdep/p2.cpp @@ -72,7 +72,7 @@ namespace O { } extern "C" { - struct L { }; + struct L { int x; }; } void h(L); // expected-note{{candidate function}} diff --git a/test/SemaCXX/extern-c.cpp b/test/SemaCXX/extern-c.cpp index 2bf9535225..dfbf38667c 100644 --- a/test/SemaCXX/extern-c.cpp +++ b/test/SemaCXX/extern-c.cpp @@ -140,7 +140,7 @@ namespace N2 { // We allow all these even though the standard says they are ill-formed. extern "C" { - struct stat {}; + struct stat {}; // expected-warning{{empty struct has size 0 in C, size 1 in C++}} void stat(struct stat); } namespace X { @@ -187,3 +187,20 @@ namespace X { extern "C" double global_var_vs_extern_c_var_2; // expected-note {{here}} } int global_var_vs_extern_c_var_2; // expected-error {{conflicts with declaration with C language linkage}} + +template struct pr5065_n1 {}; +extern "C" { + union pr5065_1 {}; // expected-warning{{empty union has size 0 in C, size 1 in C++}} + struct pr5065_2 { int: 0; }; // expected-warning{{struct has size 0 in C, size 1 in C++}} + struct pr5065_3 {}; // expected-warning{{empty struct has size 0 in C, size 1 in C++}} + struct pr5065_4 { // expected-warning{{empty struct has size 0 in C, size 1 in C++}} + struct Inner {}; // expected-warning{{empty struct has size 0 in C, size 1 in C++}} + }; + // These should not warn + class pr5065_n3 {}; + pr5065_n1 pr5065_v; + struct pr5065_n4 { void m() {} }; + struct pr5065_n5 : public pr5065_3 {}; + struct pr5065_n6 : public virtual pr5065_3 {}; +} +struct pr5065_n7 {}; diff --git a/test/SemaCXX/storage-class.cpp b/test/SemaCXX/storage-class.cpp index 74121843e5..decf1e7ac3 100644 --- a/test/SemaCXX/storage-class.cpp +++ b/test/SemaCXX/storage-class.cpp @@ -4,4 +4,4 @@ extern int PR6495b = 42; // expected-warning{{'extern' variable has an initializ extern const int PR6495c[] = {42,43,44}; extern struct Test1 {}; // expected-warning {{'extern' is not permitted on a declaration of a type}} -extern "C" struct Test0 {}; // no warning +extern "C" struct Test0 { int x; }; // no warning -- 2.40.0