[cfe-commits] r158836 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/inline.cpp

Jordan Rose jordan_rose at apple.com
Wed Jun 20 11:50:06 PDT 2012


Author: jrose
Date: Wed Jun 20 13:50:06 2012
New Revision: 158836

URL: http://llvm.org/viewvc/llvm-project?rev=158836&view=rev
Log:
Remove -Winternal-linkage-in-inline in C++.

It's very easy for anonymous external linkage to propagate in C++ through
return types and parameter types. Likewise, it's possible that a template
containing an inline function is only used with parameters that have internal
linkage. Actually diagnosing where the internal linkage comes from is fairly
difficult (both to locate and then to print nicely). Finally, since we only
have one translation unit available, we can't even prove that any of this
violates the ODR.

This warning needs better-defined behavior in C++ before it can really go in.

Rewording of the C warning (which /is/ specified by C99) coming shortly.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExpr.cpp
    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=158836&r1=158835&r2=158836&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jun 20 13:50:06 2012
@@ -3029,11 +3029,6 @@
   "%select{function|variable}0 %1 has internal linkage but is used in an "
   "inline function with external linkage">,
   InGroup<DiagGroup<"internal-linkage-in-inline"> >;
-def warn_internal_in_extern_inline_cxx : Warning<
-  "%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_convert_inline_to_static : Note<
   "use 'static' to give inline function %0 internal linkage">;
 def note_internal_decl_declared_here : Note<

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=158836&r1=158835&r2=158836&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Jun 20 13:50:06 2012
@@ -143,26 +143,21 @@
 }
 
 /// \brief Check whether we're in an extern inline function and referring to a
-/// variable or function with internal linkage.
+/// variable or function with internal linkage (C11 6.7.4p3).
 ///
-/// 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.
+/// in many cases it will not behave correctly. This is not enabled in C++ mode
+/// because the restriction language is a bit weaker (C++11 [basic.def.odr]p6)
+/// and so while there may still be user mistakes, most of the time we can't
+/// prove that there are errors.
 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; ...
+  // This is disabled under C++; there are too many ways for this to fire in
+  // contexts where the warning is a false positive, or where it is technically
+  // correct but benign.
+  if (S.getLangOpts().CPlusPlus)
+    return;
 
   // Check if this is an inlined function or method.
   FunctionDecl *Current = S.getCurFunctionDecl();
@@ -174,56 +169,19 @@
     return;
   
   // Check if the decl has internal linkage.
-  Linkage UsedLinkage = D->getLinkage();
-  switch (UsedLinkage) {
-  case NoLinkage:
-    return;
-  case InternalLinkage:
-  case UniqueExternalLinkage:
-    break;
-  case ExternalLinkage:
+  if (D->getLinkage() != InternalLinkage)
     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, and in C++ don't warn at all, since the one-definition rule is
-  // still satisfied. This function will most likely not be inlined into
+  // source file. This function will most likely not be inlined into
   // another translation unit, so it's effectively internal.
   bool IsInMainFile = S.getSourceManager().isFromMainFile(Loc);
-  if (S.getLangOpts().CPlusPlus) {
-    if (IsInMainFile)
-      return;
-
-    S.Diag(Loc, diag::warn_internal_in_extern_inline_cxx)
-      << (bool)VD << D
-      << (UsedLinkage == UniqueExternalLinkage)
-      << isa<CXXMethodDecl>(Current);
-  } else {
-    S.Diag(Loc, IsInMainFile ? diag::ext_internal_in_extern_inline
-                             : diag::warn_internal_in_extern_inline)
-      << (bool)VD << D;
-  }
+  S.Diag(Loc, IsInMainFile ? diag::ext_internal_in_extern_inline
+                           : diag::warn_internal_in_extern_inline)
+    << isa<VarDecl>(D) << D;
 
   // Suggest "static" on the inline function, if possible.
-  if (!isa<CXXMethodDecl>(Current) &&
-      !hasAnyExplicitStorageClass(Current)) {
+  if (!hasAnyExplicitStorageClass(Current)) {
     const FunctionDecl *FirstDecl = Current->getCanonicalDecl();
     SourceLocation DeclBegin = FirstDecl->getSourceRange().getBegin();
     S.Diag(DeclBegin, diag::note_convert_inline_to_static)

Modified: cfe/trunk/test/SemaCXX/inline.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/inline.cpp?rev=158836&r1=158835&r2=158836&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/inline.cpp (original)
+++ cfe/trunk/test/SemaCXX/inline.cpp Wed Jun 20 13:50:06 2012
@@ -1,110 +1,5 @@
 // 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 don't show up even when explicitly requested in C++.
-
-#pragma clang diagnostic push
-#pragma clang diagnostic warning "-Winternal-linkage-in-inline"
-
-inline int useStaticAgain () {
-  anonFunction(); // no-warning
-  return staticVar; // no-warning
-}
-
-#pragma clang diagnostic pop
-
-#endif





More information about the cfe-commits mailing list