We regard a function as 'unused' from the codegen perspective, so our warnings diverg...

gcc's unused warnings which don't get emitted if the function is referenced even in an unevaluated context
(e.g. in templates, sizeof, etc.). Also, saying that a function is 'unused' because it won't get codegen'ed
is somewhat misleading.

- Don't emit 'unused' warnings for functions that are referenced in any part of the user's code.
- A warning that an internal function/variable won't get emitted is useful though, so introduce
  -Wunneeded-internal-declaration which will warn if a function/variable with internal linkage is not
  "needed" ('used' from the codegen perspective), e.g:

  static void foo() { }

  template <int>
  void bar() {
    foo();
  }

test.cpp:1:13: warning: function 'foo' is not needed and will not be emitted
static void foo() { }
            ^

Addresses rdar://8733476.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@129794 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 1656240bc4eebfbc8e73930c699cc0bcf622ecc2..0473d1ca1f750dd6514bff2ba7e7cebb89a3178c 100644 (file)
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -227,6 +227,12 @@ private:
   /// required.
   unsigned Used : 1;
 
+  /// \brief Whether this declaration was "referenced".
+  /// The difference with 'Used' is whether the reference appears in a
+  /// evaluated context or not, e.g. functions used in uninstantiated templates
+  /// are regarded as "referenced" but not "used".
+  unsigned Referenced : 1;
+
 protected:
   /// Access - Used by C++ decls for the access specifier.
   // NOTE: VC++ treats enums as signed, avoid using the AccessSpecifier enum
@@ -261,7 +267,7 @@ protected:
   Decl(Kind DK, DeclContext *DC, SourceLocation L)
     : NextDeclInContext(0), DeclCtx(DC),
       Loc(L), DeclKind(DK), InvalidDecl(0),
-      HasAttrs(false), Implicit(false), Used(false),
+      HasAttrs(false), Implicit(false), Used(false), Referenced(false),
       Access(AS_none), PCHLevel(0), ChangedAfterLoad(false),
       IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
       HasCachedLinkage(0) 
@@ -271,7 +277,7 @@ protected:
 
   Decl(Kind DK, EmptyShell Empty)
     : NextDeclInContext(0), DeclKind(DK), InvalidDecl(0),
-      HasAttrs(false), Implicit(false), Used(false),
+      HasAttrs(false), Implicit(false), Used(false), Referenced(false),
       Access(AS_none), PCHLevel(0), ChangedAfterLoad(false),
       IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
       HasCachedLinkage(0)
@@ -408,6 +414,11 @@ public:
 
   void setUsed(bool U = true) { Used = U; }
 
+  /// \brief Whether this declaration was referenced.
+  bool isReferenced() const;
+
+  void setReferenced(bool R = true) { Referenced = R; }
+
   /// \brief Determine the availability of the given declaration.
   ///
   /// This routine will determine the most restrictive availability of

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index c8465508c8b643a86f5be138e63f7a043c42ddb1..15f3dbd4640d397b89ab6bd56890906726d9dde9 100644 (file)
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -154,6 +154,8 @@ def UnusedLabel : DiagGroup<"unused-label">;
 def UnusedParameter : DiagGroup<"unused-parameter">;
 def UnusedValue    : DiagGroup<"unused-value">;
 def UnusedVariable : DiagGroup<"unused-variable">;
+def UnneededInternalDecl : DiagGroup<"unneeded-internal-declaration">;
+def UnneededMemberFunction : DiagGroup<"unneeded-member-function">;
 def UsedButMarkedUnused : DiagGroup<"used-but-marked-unused">;
 def ReadOnlySetterAttrs : DiagGroup<"readonly-setter-attrs">;
 def Reorder : DiagGroup<"reorder">;
@@ -198,11 +200,16 @@ def Conversion : DiagGroup<"conversion",
                             BoolConversions]>,
                  DiagCategory<"Value Conversion Issue">;
 
+def Unneeded : DiagGroup<"unneeded",
+                         [UnneededInternalDecl
+                       //,UnneededMemberFunction (clean-up llvm before enabling)
+                          ]>;
+
 def Unused : DiagGroup<"unused",
                        [UnusedArgument, UnusedFunction, UnusedLabel,
                         // UnusedParameter, (matches GCC's behavior)
                         // UnusedMemberFunction, (clean-up llvm before enabling)
-                        UnusedValue, UnusedVariable]>,
+                        UnusedValue, UnusedVariable, Unneeded]>,
                         DiagCategory<"Unused Entity Issue">;
 
 // Format settings.

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 3ca45d4cf35f73336a775887a60ec941024d26f0..ce592a3c953d2ccfea962dd91115475d4fb48e37 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -116,6 +116,12 @@ def warn_unused_member_function : Warning<"unused member function %0">,
   InGroup<UnusedMemberFunction>, DefaultIgnore;
 def warn_used_but_marked_unused: Warning<"%0 was marked unused but was used">,
   InGroup<UsedButMarkedUnused>, DefaultIgnore;
+def warn_unneeded_internal_decl : Warning<
+  "%select{function|variable}0 %1 is not needed and will not be emitted">,
+  InGroup<UnneededInternalDecl>, DefaultIgnore;
+def warn_unneeded_member_function : Warning<
+  "member function %0 is not needed and will not be emitted">,
+  InGroup<UnneededMemberFunction>, DefaultIgnore;
 
 def warn_parameter_size: Warning<
   "%0 is a large (%1 bytes) pass-by-value argument; "

diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 673b718ba0ed866f76acf8300b63d2e065522725..41a036142504ca436503569eb235bbbbec118f71 100644 (file)
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -242,6 +242,18 @@ bool Decl::isUsed(bool CheckUsedAttr) const {
   return false; 
 }
 
+bool Decl::isReferenced() const { 
+  if (Referenced)
+    return true;
+
+  // Check redeclarations.
+  for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I)
+    if (I->Referenced)
+      return true;
+
+  return false; 
+}
+
 /// \brief Determine the availability of the given declaration based on
 /// the target platform.
 ///

diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index a83722d82452c2e026bc0b325bfd49a667a0a278..403cf6246cfed38d3076d1f0fb36d0fb6681dc66 100644 (file)
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -473,16 +473,30 @@ void Sema::ActOnEndOfTranslationUnit() {
           DiagD = FD;
         if (DiagD->isDeleted())
           continue; // Deleted functions are supposed to be unused.
-        Diag(DiagD->getLocation(),
-             isa<CXXMethodDecl>(DiagD) ? diag::warn_unused_member_function
-                                       : diag::warn_unused_function)
-              << DiagD->getDeclName();
+        if (DiagD->isReferenced()) {
+          if (isa<CXXMethodDecl>(DiagD))
+            Diag(DiagD->getLocation(), diag::warn_unneeded_member_function)
+                  << DiagD->getDeclName();
+          else
+            Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
+                  << /*function*/0 << DiagD->getDeclName();
+        } else {
+          Diag(DiagD->getLocation(),
+               isa<CXXMethodDecl>(DiagD) ? diag::warn_unused_member_function
+                                         : diag::warn_unused_function)
+                << DiagD->getDeclName();
+        }
       } else {
         const VarDecl *DiagD = cast<VarDecl>(*I)->getDefinition();
         if (!DiagD)
           DiagD = cast<VarDecl>(*I);
-        Diag(DiagD->getLocation(), diag::warn_unused_variable)
-              << DiagD->getDeclName();
+        if (DiagD->isReferenced()) {
+          Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
+                << /*variable*/1 << DiagD->getDeclName();
+        } else {
+          Diag(DiagD->getLocation(), diag::warn_unused_variable)
+                << DiagD->getDeclName();
+        }
       }
     }
 

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 9415673a774750c9704f435fcdcd969872b716cc..8eb0177046da6bdc03958a8e9750b075c598d75e 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9885,6 +9885,8 @@ Sema::PopExpressionEvaluationContext() {
 void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
   assert(D && "No declaration?");
 
+  D->setReferenced();
+
   if (D->isUsed(false))
     return;
 

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 3fe5fd5c952852362a571815d112d4f04828b5f5..0ff7ff4d4064bd81bca0a7318c89f118e245ffbb 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -298,8 +298,10 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) {
 
   Var->setAccess(D->getAccess());
   
-  if (!D->isStaticDataMember())
+  if (!D->isStaticDataMember()) {
     Var->setUsed(D->isUsed(false));
+    Var->setReferenced(D->isReferenced());
+  }
   
   // FIXME: In theory, we could have a previous declaration for variables that
   // are not static data members.

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 5b4a64eeb5c9aade6f1b2b83cd13fe36aeec23b6..889ef7324c839e9198882c8367b779aed0849b3a 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -214,6 +214,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
   }
   D->setImplicit(Record[Idx++]);
   D->setUsed(Record[Idx++]);
+  D->setReferenced(Record[Idx++]);
   D->setAccess((AccessSpecifier)Record[Idx++]);
   D->setPCHLevel(Record[Idx++] + (F.Type <= ASTReader::PCH));
 }

diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index 1306ffda6dd1da6553b5dd57ed54ace053aac659..b3b55f584aecf5dc224130a292342bcc05f1db46 100644 (file)
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -141,6 +141,7 @@ void ASTDeclWriter::VisitDecl(Decl *D) {
     Writer.WriteAttributes(D->getAttrs(), Record);
   Record.push_back(D->isImplicit());
   Record.push_back(D->isUsed(false));
+  Record.push_back(D->isReferenced());
   Record.push_back(D->getAccess());
   Record.push_back(D->getPCHLevel());
 }
@@ -1133,6 +1134,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                       // HasAttrs
   Abv->Add(BitCodeAbbrevOp(0));                       // isImplicit
   Abv->Add(BitCodeAbbrevOp(0));                       // isUsed
+  Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(AS_none));                 // C++ AccessSpecifier
   Abv->Add(BitCodeAbbrevOp(0));                       // PCH level
 

diff --git a/test/Sema/warn-unused-function.c b/test/Sema/warn-unused-function.c
index 5bbcf18a623d9d760165f40868b1b8c5ecc6dad2..8f4cdcba881e4046af776e356fe4dc400adfe97e 100644 (file)
--- a/test/Sema/warn-unused-function.c
+++ b/test/Sema/warn-unused-function.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wunused-function -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-function -Wunneeded-internal-declaration -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall %s
 
@@ -6,7 +6,7 @@ void foo() {}
 static void f2() {} 
 static void f1() {f2();} // expected-warning{{unused}}
 
-static int f0() { return 17; } // expected-warning{{unused}}
+static int f0() { return 17; } // expected-warning{{not needed and will not be emitted}}
 int x = sizeof(f0());
 
 static void f3();
@@ -46,7 +46,7 @@ static void f12(void) { }  // expected-warning{{unused}}
 static void f12(void);
 
 // PR7923
-static void unused(void) { unused(); }  // expected-warning{{unused}}
+static void unused(void) { unused(); }  // expected-warning{{not needed and will not be emitted}}
 
 // rdar://8728293
 static void cleanupMalloc(char * const * const allocation) { }

diff --git a/test/SemaCXX/warn-unused-filescoped.cpp b/test/SemaCXX/warn-unused-filescoped.cpp
index c32acb0b48be90e57db39661f074b2efb95fe213..dbff4b0e68c187bdf5d7bd0606aa7e8d8cc353fd 100644 (file)
--- a/test/SemaCXX/warn-unused-filescoped.cpp
+++ b/test/SemaCXX/warn-unused-filescoped.cpp
@@ -78,3 +78,12 @@ namespace test4 {
   void test(A a); // expected-warning {{unused function}}
   extern "C" void test4(A a);
 }
+
+namespace rdar8733476 {
+  static void foo() { } // expected-warning {{not needed and will not be emitted}}
+
+  template <int>
+  void bar() {
+    foo();
+  }
+}