[cfe-commits] r158683 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/inline.c test/SemaCXX/inline.cpp
Jordan Rose
jordan_rose at apple.com
Mon Jun 18 15:09:19 PDT 2012
Author: jrose
Date: Mon Jun 18 17:09:19 2012
New Revision: 158683
URL: http://llvm.org/viewvc/llvm-project?rev=158683&view=rev
Log:
Support -Winternal-linkage-in-inline in C++ code.
This includes treating anonymous namespaces like internal linkage, and allowing
const variables to be used even if internal. The whole thing's been broken out
into a separate function to avoid nested ifs.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/inline.c
cfe/trunk/test/SemaCXX/inline.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=158683&r1=158682&r2=158683&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jun 18 17:09:19 2012
@@ -2997,17 +2997,19 @@
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<DiagGroup<"internal-linkage-in-inline"> >;
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<DiagGroup<"internal-linkage-in-inline"> >;
-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">,
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=158683&r1=158682&r2=158683&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jun 18 17:09:19 2012
@@ -130,6 +130,103 @@
<< 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<VarDecl>(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<CXXMethodDecl>(Current);
+
+ // Suggest "static" on the inline function, if possible.
+ if (!isa<CXXMethodDecl>(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 @@
if (D->hasAttr<UnusedAttr>())
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<FunctionDecl>(D) << D << isa<CXXMethodDecl>(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;
}
Modified: cfe/trunk/test/Sema/inline.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline.c?rev=158683&r1=158682&r2=158683&view=diff
==============================================================================
--- cfe/trunk/test/Sema/inline.c (original)
+++ cfe/trunk/test/Sema/inline.c Mon Jun 18 17:09:19 2012
@@ -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 @@
// 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
}
Modified: cfe/trunk/test/SemaCXX/inline.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/inline.cpp?rev=158683&r1=158682&r2=158683&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/inline.cpp (original)
+++ cfe/trunk/test/SemaCXX/inline.cpp Mon Jun 18 17:09:19 2012
@@ -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
More information about the cfe-commits
mailing list