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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
