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.)

llvm-svn: 158898

2 files changed, 29 insertions, 7 deletions

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index de0de89b833..e50dfca8f53 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -172,13 +172,21 @@ static void diagnoseUseOfInternalDeclInInlineFunction(Sema &S,
   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)) {
diff --git a/clang/test/Sema/inline.c b/clang/test/Sema/inline.c
index 6c95a7a14b4..c27c00efaad 100644
--- a/clang/test/Sema/inline.c
+++ b/clang/test/Sema/inline.c
@@ -26,6 +26,20 @@ static inline int useStaticFromStatic () {
   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.