[cfe-commits] [PATCH] Unused function warning

[Douglas Gregor]

On Feb 9, 2010, at 5:51 PM, Tanya Lattner wrote:

> Sorry, for the delay. I've implemented your suggested changes and added a FIXME for the static functions declared but not defined. I've attached the new patch.

Thanks. It looks good modulo two comments below. Please go ahead and commit when you're ready.

> However, please note that there seems to be a decl merging bug because a warning is triggered for your suggested test case:
> inline static void f4();
> void f4() { } // inline, should not complain
> 
> Clang seems to believe that f4 is not inlined. (!NewFD->isInlined()  returns true)

Ugh, that's bad but it isn't your problem to fix it. You can just FIXME that particular part of the test and we'll tackle the isInlined() breakage here.

Index: include/clang/Frontend/PCHReader.h
===================================================================
--- include/clang/Frontend/PCHReader.h	(revision 95744)
+++ include/clang/Frontend/PCHReader.h	(working copy)
@@ -306,6 +306,10 @@
   /// \brief The set of tentative definitions stored in the the PCH
   /// file.
   llvm::SmallVector<uint64_t, 16> TentativeDefinitions;
+      
+  /// \brief The set of tentative definitions stored in the the PCH
+  /// file.
+  llvm::SmallVector<uint64_t, 16> UnusedStaticFuncs;
 
This comment should be updated.

+  // Remove functions that turned out to be used.
+  for (std::vector<FunctionDecl*>::iterator
+       F = UnusedStaticFuncs.begin();
+       F != UnusedStaticFuncs.end();) {
+    if ((*F)->isUsed())
+      UnusedStaticFuncs.erase(F);
+    else 
+      ++F;
+  }

F = UnusedStaticFuncs.erase(F);

would be slightly better, since a sufficiently smart debugging STL would catch that F has been invalidated by the erase. (Such a thing would show up under expensive checks, but would never cause a real problem in practice).

More importantly, this loop is actually quadratic. It could be linearized by replacing it with this magic incantation:

	UnusedStaticFuncs.erase(std::remove_if(UnusedStaticFunctions.begin(), UnusedStaticFunctions.end(), std::mem_fun(&FunctionDecl::isUsed)), UnusedStaticFunctions.end());

	- Doug