diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 3b7e0522687e..4507b2fd572e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2997,17 +2997,19 @@ def warn_undefined_internal : Warning< def note_used_here : Note<"used here">; def warn_internal_in_extern_inline : ExtWarn< - "%select{function|variable}0 %1 has internal linkage but is used in an " - "inline %select{function|method}2 with external linkage">, + "%select{function|variable}0 %1 %select{has internal linkage|is in an anonymous" + " namespace}2 but is used in an inline %select{function|method}3 with" + " external linkage">, InGroup >; def ext_internal_in_extern_inline : Extension< - "%select{function|variable}0 %1 has internal linkage but is used in an " - "inline %select{function|method}2 with external linkage">, + "%select{function|variable}0 %1 %select{has internal linkage|is in an anonymous" + " namespace}2 but is used in an inline %select{function|method}3 with" + " external linkage">, InGroup >; -def note_internal_decl_declared_here : Note< - "%0 declared here">; def note_convert_inline_to_static : Note< "use 'static' to give inline function %0 internal linkage">; +def note_internal_decl_declared_here : Note< + "%0 declared here">; def warn_redefinition_of_typedef : ExtWarn< "redefinition of typedef %0 is a C11 feature">, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index d9950949ea85..c9b1a694d013 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -130,6 +130,103 @@ void Sema::NoteDeletedFunction(FunctionDecl *Decl) { << 1 << Decl->isDeleted(); } +/// \brief Determine whether a FunctionDecl was ever declared with an +/// explicit storage class. +static bool hasAnyExplicitStorageClass(const FunctionDecl *D) { + for (FunctionDecl::redecl_iterator I = D->redecls_begin(), + E = D->redecls_end(); + I != E; ++I) { + if (I->getStorageClassAsWritten() != SC_None) + return true; + } + return false; +} + +/// \brief Check whether we're in an extern inline function and referring to a +/// variable or function with internal linkage. +/// +/// This also applies to anonymous-namespaced objects, which are effectively +/// internal. +/// This is only a warning because we used to silently accept this code, but +/// most likely it will not do what the user intends. +static void diagnoseUseOfInternalDeclInInlineFunction(Sema &S, + const NamedDecl *D, + SourceLocation Loc) { + // C11 6.7.4p3: An inline definition of a function with external linkage... + // shall not contain a reference to an identifier with internal linkage. + // C++11 [basic.def.odr]p6: ...in each definition of D, corresponding names, + // looked up according to 3.4, shall refer to an entity defined within the + // definition of D, or shall refer to the same entity, after overload + // resolution (13.3) and after matching of partial template specialization + // (14.8.3), except that a name can refer to a const object with internal or + // no linkage if the object has the same literal type in all definitions of + // D, and the object is initialized with a constant expression (5.19), and + // the value (but not the address) of the object is used, and the object has + // the same value in all definitions of D; ... + + // Check if this is an inlined function or method. + FunctionDecl *Current = S.getCurFunctionDecl(); + if (!Current) + return; + if (!Current->isInlined()) + return; + if (Current->getLinkage() != ExternalLinkage) + return; + + // Check if the decl has internal linkage. + Linkage UsedLinkage = D->getLinkage(); + switch (UsedLinkage) { + case NoLinkage: + return; + case InternalLinkage: + case UniqueExternalLinkage: + break; + case ExternalLinkage: + return; + } + + // Check C++'s exception for const variables. This is in the standard + // because in C++ const variables have internal linkage unless + // explicitly declared extern. + // Note that we don't do any of the cross-TU checks, and this code isn't + // even particularly careful about checking if the variable "has the + // same value in all definitions" of the inline function. It just does a + // sanity check to make sure there is an initializer at all. + // FIXME: We should still be checking to see if we're using a constant + // as a glvalue anywhere, but we don't have the necessary information to + // do that here, and as long as this is a warning and not a hard error + // it's not appropriate to change the semantics of the program (i.e. + // by having BuildDeclRefExpr use VK_RValue for constants like these). + const VarDecl *VD = dyn_cast(D); + if (VD && S.getLangOpts().CPlusPlus) + if (VD->getType().isConstant(S.getASTContext()) && VD->getAnyInitializer()) + return; + + // Don't warn unless -pedantic is on if the inline function is in the main + // source file. These functions will most likely not be inlined into another + // translation unit, so they're effectively internal. + bool IsInMainFile = S.getSourceManager().isFromMainFile(Loc); + S.Diag(Loc, IsInMainFile ? diag::ext_internal_in_extern_inline + : diag::warn_internal_in_extern_inline) + << (bool)VD + << D + << (UsedLinkage == UniqueExternalLinkage) + << isa(Current); + + // Suggest "static" on the inline function, if possible. + if (!isa(Current) && + !hasAnyExplicitStorageClass(Current)) { + const FunctionDecl *FirstDecl = Current->getCanonicalDecl(); + SourceLocation DeclBegin = FirstDecl->getSourceRange().getBegin(); + S.Diag(DeclBegin, diag::note_convert_inline_to_static) + << Current << FixItHint::CreateInsertion(DeclBegin, "static "); + } + + S.Diag(D->getCanonicalDecl()->getLocation(), + diag::note_internal_decl_declared_here) + << D; +} + /// \brief Determine whether the use of this declaration is valid, and /// emit any corresponding diagnostics. /// @@ -183,42 +280,7 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc, if (D->hasAttr()) Diag(Loc, diag::warn_used_but_marked_unused) << D->getDeclName(); - // Warn if we're in an extern inline function referring to a decl - // with internal linkage. (C99 6.7.4p3) - // FIXME: This is not explicitly forbidden in C++, but it's not clear - // what the correct behavior is. We should probably still have a warning. - // (However, in C++ const variables have internal linkage by default, while - // functions still have external linkage by default, so this warning becomes - // very noisy.) - if (!getLangOpts().CPlusPlus) { - if (FunctionDecl *Current = getCurFunctionDecl()) { - if (Current->isInlined() && Current->getLinkage() > InternalLinkage) { - if (D->getLinkage() == InternalLinkage) { - // We won't warn by default if the inline function is in the main - // source file; in these cases it is almost certain that the inlining - // will only occur in this file, even if there is an external - // declaration as well. - bool IsFromMainFile = getSourceManager().isFromMainFile(Loc); - Diag(Loc, IsFromMainFile ? diag::ext_internal_in_extern_inline - : diag::warn_internal_in_extern_inline) - << !isa(D) << D << isa(Current); - - // If the user didn't explicitly specify a storage class, - // suggest adding "static" to fix the problem. - const FunctionDecl *FirstDecl = Current->getCanonicalDecl(); - if (FirstDecl->getStorageClassAsWritten() == SC_None) { - SourceLocation DeclBegin = FirstDecl->getSourceRange().getBegin(); - Diag(DeclBegin, diag::note_convert_inline_to_static) - << Current << FixItHint::CreateInsertion(DeclBegin, "static "); - } - - Diag(D->getCanonicalDecl()->getLocation(), - diag::note_internal_decl_declared_here) - << D; - } - } - } - } + diagnoseUseOfInternalDeclInInlineFunction(*this, D, Loc); return false; } diff --git a/clang/test/Sema/inline.c b/clang/test/Sema/inline.c index 23eedd6b2265..99df8b1106b8 100644 --- a/clang/test/Sema/inline.c +++ b/clang/test/Sema/inline.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s #if defined(INCLUDE) // ------- @@ -40,7 +41,7 @@ int d(inline int a); // expected-error{{'inline' can only appear on functions}} // Check that the warnings from the "header file" aren't on by default in // the main source file. -inline int useStaticMain () { +inline int useStaticMainFile () { staticFunction(); // no-warning return staticVar; // no-warning } diff --git a/clang/test/SemaCXX/inline.cpp b/clang/test/SemaCXX/inline.cpp index e569300faf77..9ef0229c990a 100644 --- a/clang/test/SemaCXX/inline.cpp +++ b/clang/test/SemaCXX/inline.cpp @@ -1,5 +1,110 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +#if defined(INCLUDE) +// ------- +// This section acts like a header file. +// ------- + +// Check the use of static variables in non-static inline functions. +static int staticVar; // expected-note + {{'staticVar' declared here}} +static int staticFunction(); // expected-note + {{'staticFunction' declared here}} +const int constVar = 0; // no-warning + +namespace { + int anonVar; // expected-note + {{'anonVar' declared here}} + int anonFunction(); // expected-note + {{'anonFunction' declared here}} + const int anonConstVar = 0; // no-warning + + class Anon { + public: + static int var; // expected-note + {{'var' declared here}} + static const int constVar = 0; // no-warning + }; +} + +inline void useStatic() { // expected-note + {{use 'static' to give inline function 'useStatic' internal linkage}} + staticFunction(); // expected-warning{{function 'staticFunction' has internal linkage but is used in an inline function with external linkage}} + (void)staticVar; // expected-warning{{variable 'staticVar' has internal linkage but is used in an inline function with external linkage}} + anonFunction(); // expected-warning{{function 'anonFunction' is in an anonymous namespace but is used in an inline function with external linkage}} + (void)anonVar; // expected-warning{{variable 'anonVar' is in an anonymous namespace but is used in an inline function with external linkage}} + (void)Anon::var; // expected-warning{{variable 'var' is in an anonymous namespace but is used in an inline function with external linkage}} + + (void)constVar; // no-warning + (void)anonConstVar; // no-warning + (void)Anon::constVar; // no-warning +} + +extern inline int useStaticFromExtern() { // no suggestions + staticFunction(); // expected-warning{{function 'staticFunction' has internal linkage but is used in an inline function with external linkage}} + return staticVar; // expected-warning{{variable 'staticVar' has internal linkage but is used in an inline function with external linkage}} +} + +class A { +public: + static inline int useInClass() { // no suggestions + return staticFunction(); // expected-warning{{function 'staticFunction' has internal linkage but is used in an inline method with external linkage}} + } + inline int useInInstance() { // no suggestions + return staticFunction(); // expected-warning{{function 'staticFunction' has internal linkage but is used in an inline method with external linkage}} + } +}; + +static inline void useStaticFromStatic () { + // No warnings. + staticFunction(); + (void)staticVar; + (void)constVar; + anonFunction(); + (void)anonVar; + (void)anonConstVar; + (void)Anon::var; + (void)Anon::constVar; +} + +namespace { + inline void useStaticFromAnon() { + // No warnings. + staticFunction(); + (void)staticVar; + (void)constVar; + anonFunction(); + (void)anonVar; + (void)anonConstVar; + (void)Anon::var; + (void)Anon::constVar; + } +} + +#else +// ------- +// This is the main source file. +// ------- + +#define INCLUDE +#include "inline.cpp" + // Check that we don't allow illegal uses of inline // (checking C++-only constructs here) struct c {inline int a;}; // expected-error{{'inline' can only appear on functions}} + +// Check that the warnings from the "header file" aren't on by default in +// the main source file. + +inline int useStaticMainFile () { + anonFunction(); // no-warning + return staticVar; // no-warning +} + +// Check that the warnings show up when explicitly requested. + +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Winternal-linkage-in-inline" + +inline int useStaticAgain () { // expected-note 2 {{use 'static' to give inline function 'useStaticAgain' internal linkage}} + anonFunction(); // expected-warning{{function 'anonFunction' is in an anonymous namespace but is used in an inline function with external linkage}} + return staticVar; // expected-warning{{variable 'staticVar' has internal linkage but is used in an inline function with external linkage}} +} + +#pragma clang diagnostic pop + +#endif