[cfe-commits] r129794 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/AST/DeclBase.cpp lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaTemplateInstantiateDe

Nico Weber thakis at chromium.org
Sat Apr 23 12:51:01 PDT 2011


Hi Argyrios,

this breaks the chromium webkit build. The build now complains about
one unused function; clang didn't use to complain before. Maybe
-Wno-unused-function should imply -Wno-unneeded-internal-declaration?

(It's not a big deal for us in practice – I'll add
-Wno-unneeded-internal-declaration to our build configuration. We're
building with -Wno-unused-function because the linker will drop unused
functions anyway and that warning was a little too noisy. In this
case, the new warning is also mostly noise – it's warning about a
static function defined in a header file that gets included in a cpp
file which uses that static function only if certain preprocessor
defines are set. I could include the header only under these
circumstances, but that seems like unnecessary busywork.)

Just some random feedback; make of it what you will :-)

Nico

On Tue, Apr 19, 2011 at 12:51 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> Author: akirtzidis
> Date: Tue Apr 19 14:51:10 2011
> New Revision: 129794
>
> URL: http://llvm.org/viewvc/llvm-project?rev=129794&view=rev
> Log:
> We regard a function as 'unused' from the codegen perspective, so our warnings diverge from
> 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.
>
> Modified:
>    cfe/trunk/include/clang/AST/DeclBase.h
>    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/AST/DeclBase.cpp
>    cfe/trunk/lib/Sema/Sema.cpp
>    cfe/trunk/lib/Sema/SemaExpr.cpp
>    cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>    cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>    cfe/trunk/test/Sema/warn-unused-function.c
>    cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 14:51:10 2011
> @@ -227,6 +227,12 @@
>   /// 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 @@
>   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 @@
>
>   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 @@
>
>   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
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Apr 19 14:51:10 2011
> @@ -154,6 +154,8 @@
>  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 @@
>                             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.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 19 14:51:10 2011
> @@ -116,6 +116,12 @@
>   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; "
>
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Apr 19 14:51:10 2011
> @@ -242,6 +242,18 @@
>   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.
>  ///
>
> Modified: cfe/trunk/lib/Sema/Sema.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Tue Apr 19 14:51:10 2011
> @@ -473,16 +473,30 @@
>           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();
> +        }
>       }
>     }
>
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Apr 19 14:51:10 2011
> @@ -9885,6 +9885,8 @@
>  void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
>   assert(D && "No declaration?");
>
> +  D->setReferenced();
> +
>   if (D->isUsed(false))
>     return;
>
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Apr 19 14:51:10 2011
> @@ -298,8 +298,10 @@
>
>   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.
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue Apr 19 14:51:10 2011
> @@ -214,6 +214,7 @@
>   }
>   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));
>  }
>
> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Tue Apr 19 14:51:10 2011
> @@ -141,6 +141,7 @@
>     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 @@
>   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
>
>
> Modified: cfe/trunk/test/Sema/warn-unused-function.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unused-function.c?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/warn-unused-function.c (original)
> +++ cfe/trunk/test/Sema/warn-unused-function.c Tue Apr 19 14:51:10 2011
> @@ -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 @@
>  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);
>
>  // 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) { }
>
> Modified: cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=129794&r1=129793&r2=129794&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Tue Apr 19 14:51:10 2011
> @@ -78,3 +78,12 @@
>   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();
> +  }
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>




More information about the cfe-commits mailing list