[cfe-commits] r158898 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/Sema/inline.c

[Jordan Rose]

Author: jrose
Date: Thu Jun 21 00:54:50 2012
New Revision: 158898

URL: http://llvm.org/viewvc/llvm-project?rev=158898&view=rev
Log:
Don't warn for -Wstatic-in-inline if the used function is also inline.

Also, don't warn if the used function is __attribute__((const)), in which case
it's not supposed to use global variables anyway.

The inline-in-inline thing is a heuristic, and one that's possibly incorrect
fairly often because the function being inlined could definitely use global
variables. However, even some C standard library functions are written using
other (trivial) static-inline functions in the headers, and we definitely don't
want to be warning on that (or on anything that /uses/ these trivial inline
functions). So we're using "inlined" as a marker for "fairly trivial".

(Note that __attribute__((pure)) does /not/ guarantee safety like ((const),
because ((const)) does not guarantee that global variables are not being used,
and the warning is about globals not being shared across TUs.)

Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/inline.c

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=158898&r1=158897&r2=158898&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jun 21 00:54:50 2012
@@ -172,13 +172,21 @@
   if (D->getLinkage() != InternalLinkage)
     return;
 
-  // Don't warn unless -pedantic is on if the inline function is in the main
-  // 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);
-  S.Diag(Loc, IsInMainFile ? diag::ext_internal_in_extern_inline
-                           : diag::warn_internal_in_extern_inline)
-    << isa<VarDecl>(D) << D;
+  // Downgrade from ExtWarn to Extension if
+  //  (1) the supposedly external inline function is in the main file,
+  //      and probably won't be included anywhere else.
+  //  (2) the thing we're referencing is a pure function.
+  //  (3) the thing we're referencing is another inline function.
+  // This last can give us false negatives, but it's better than warning on
+  // wrappers for simple C library functions.
+  const FunctionDecl *UsedFn = dyn_cast<FunctionDecl>(D);
+  bool DowngradeWarning = S.getSourceManager().isFromMainFile(Loc);
+  if (!DowngradeWarning && UsedFn)
+    DowngradeWarning = UsedFn->isInlined() || UsedFn->hasAttr<ConstAttr>();
+
+  S.Diag(Loc, DowngradeWarning ? diag::ext_internal_in_extern_inline
+                               : diag::warn_internal_in_extern_inline)
+    << /*IsVar=*/!UsedFn << D;
 
   // Suggest "static" on the inline function, if possible.
   if (!hasAnyExplicitStorageClass(Current)) {

Modified: cfe/trunk/test/Sema/inline.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline.c?rev=158898&r1=158897&r2=158898&view=diff
==============================================================================
--- cfe/trunk/test/Sema/inline.c (original)
+++ cfe/trunk/test/Sema/inline.c Thu Jun 21 00:54:50 2012
@@ -26,6 +26,20 @@
   return staticVar; // no-warning
 }
 
+extern inline int useStaticInlineFromExtern () {
+  // Heuristic: if the function we're using is also inline, don't warn.
+  // This can still be wrong (in this case, we end up inlining calls to
+  // staticFunction and staticVar) but this got very noisy even using
+  // standard headers.
+  return useStaticFromStatic(); // no-warning
+}
+
+static int constFunction() __attribute__((const));
+
+inline int useConst () {
+  return constFunction(); // no-warning
+}
+
 #else
 // -------
 // This is the main source file.