On Jan 26, 2010, at 2:32 PM, Tanya Lattner wrote:
> I've attached a patch to implement warning of unused functions (includes a
> test case). Please let me know what you think.
I like this warning, thanks for working on it. A couple comments:
Index: test/Sema/warn-unused-function.c
===================================================================
--- test/Sema/warn-unused-function.c (revision 0)
+++ test/Sema/warn-unused-function.c (revision 0)
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-function -verify %s
+
+void foo() {}
+
+static void f1() {} // expected-warning{{unused}}
'twould be good to verify that we don't get warnings for a static function that
*is* used.
+def warn_unused_function : Warning<"unused function %0">,
+ InGroup<UnusedFunction>, DefaultIgnore;
GCC's -Wunused-function has a bit more behavior than is in your patch.
Obviously, you don't have to implement it all, but please leave FIXMEs for
cases that aren't handled (static functions declared but not defined, C++
functions in anonymous namespaces).
+
+ for (DeclContext::decl_iterator
+ D = Context.getTranslationUnitDecl()->decls_begin(),
+ DEnd = Context.getTranslationUnitDecl()->decls_end();
+ D != DEnd;
+ ++D) {
+ // Check for unused functions.
+ if (FunctionDecl *FD = dyn_cast<FunctionDecl>(*D)) {
+ if (!FD->isUsed() && !FD->hasAttr<UnusedAttr>()
+ && (FD->getStorageClass() == FunctionDecl::Static)) {
+ Diag(FD->getLocation(), diag::warn_unused_function) <<
FD->getDeclName();
+ }
+ }
+ }
This is going to be expensive, because it's walking the entire translation
unit. When precompiled headers are involved, it gets *really* expensive,
because it will end up deserializing every top-level declaration to perform
this walk.
Instead, I suggest that ActOnFunctionDef recognize when we're defining a static
function (that has not yet been used) and put it into a list of such functions.
Whenever a static function is marked used for the first time, we remove it from
that list. At the end of the translation unit, just walk that list and complain
about everything on it. The most annoying part about this is that the list will
need to be saved to the PCH file, just like tentative definitions are :(
There are two more cases to think about. First, we shouldn't warn about inline
static functions. Second, we should figure out what using one of these static
functions as an unevaluated operand should silence the warning. For example:
static int f0() { return 17; }
int x = sizeof(f0());
The "used" bit on f0 won't get set, so I'm guessing we'll end up warning about
"f0" being unused. The list-of-unused-static-functions approach I mentioned
above could avoid this problem, if we don't want the warning there. I don't
really have a strong opinion on the matter.
I wonder if this warning should have a Fix-It that removes the function
entirely :)
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
