[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 17:11:41 PDT 2011


On Sat, Apr 23, 2011 at 1:13 PM, Nico Weber <thakis at chromium.org> wrote:
> …and I can't just replace -Wno-unused-function with
> -Wno-unneeded-internal-declaration, because clang still warns about
>
> thakis-macbookpro:src thakis$ cat test.cc
> namespace {
> void f() {}
> }

I filed http://llvm.org/pr9786 for this. Other than that, I think
nothing else needs to be done here.

>
> while gcc doesn't. So -Wunused-function in gcc and clang are still
> different. Warnings about unused functions in the unnamed namespace
> should probably be moved to Wunneeded-internal-declaration.
>
> Nico
>
> On Sat, Apr 23, 2011 at 12:51 PM, Nico Weber <thakis at chromium.org> wrote:
>> 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