[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 13:13:18 PDT 2011
…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() {}
}
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