Hi,
I would like to improve -Wunused-member-function to detect unused private
methods, similarly to how -Wunused-private-fields works.
I think clang should be able to flag a method if 1) it is unused, 2) all the
methods of the class are defined in the TU, and 3) any of the following
conditions holds:
- The method is private.
- The method is protected and the class is final.
- The method is public and the class is in an anonymous namespace.
I have a simple implementation in attach that can handle the first case
(private methods) only.
I'm not very happy with it, though. In particular I would like to move the
logic somewhere else, so that we can reuse it from Codegen. And right now
I'm not caching things properly. Any suggestions to where this code
belongs? Should it go directly to Decl? (but that would imply adding a few
fields for cache purposes).
The second part of the plan is to mark all methods described previously
(well, the used ones) with internal linkage. In my naive view it seems fine,
but is this legal per the standard? I think it can be an important
optimization, since then LLVM can more freely inline and remove the symbols
altogether.
Any comments and suggestions are welcomed!
Thanks,
Nuno
P.S.: I run the attached patch over the LLVM codebase and I already fixed a
bunch of cases it detected (but left many still). So big code bases will
certainly benefit from this analysis. Moreover, removing unused decls
triggered more -Wunused-private-fields warnings.
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp (revision 204559)
+++ lib/Sema/Sema.cpp (working copy)
@@ -361,11 +361,26 @@
return CK_Invalid;
}
+
+typedef llvm::DenseMap<const CXXRecordDecl*, bool> RecordCompleteMap;
+
+static bool IsRecordFullyDefined(const CXXRecordDecl *RD,
+ RecordCompleteMap &RecordsComplete,
+ RecordCompleteMap &MNCComplete);
+
/// \brief Used to prune the decls of Sema's UnusedFileScopedDecls vector.
static bool ShouldRemoveFromUnused(Sema *SemaRef, const DeclaratorDecl *D) {
if (D->getMostRecentDecl()->isUsed())
return true;
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
+ RecordCompleteMap RecordsComplete;
+ RecordCompleteMap MNCComplete;
+ if (MD->getAccess() == AS_private &&
+ IsRecordFullyDefined(MD->getParent(), RecordsComplete, MNCComplete))
+ return false;
+ }
+
if (D->isExternallyVisible())
return true;
@@ -500,8 +515,6 @@
}
-typedef llvm::DenseMap<const CXXRecordDecl*, bool> RecordCompleteMap;
-
/// \brief Returns true, if all methods and nested classes of the given
/// CXXRecordDecl are defined in this translation unit.
///
@@ -750,14 +763,28 @@
// If there were errors, disable 'unused' warnings since they will mostly be
// noise.
if (!Diags.hasErrorOccurred()) {
+ llvm::SmallSet<const Decl*, 16> SeenDecls;
+
// Output warning for unused file scoped decls.
for (UnusedFileScopedDeclsType::iterator
I = UnusedFileScopedDecls.begin(ExternalSource),
E = UnusedFileScopedDecls.end(); I != E; ++I) {
- if (ShouldRemoveFromUnused(this, *I))
+ if (ShouldRemoveFromUnused(this, *I) || !SeenDecls.insert(*I))
continue;
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(*I)) {
+ const FunctionDecl *First = FD->getFirstDecl();
+ if (FD != First && !SeenDecls.insert(First))
+ continue;
+ }
+
+ if (const VarDecl *VD = dyn_cast<VarDecl>(*I)) {
+ const VarDecl *First = VD->getFirstDecl();
+ if (VD != First && !SeenDecls.insert(First))
+ continue;
+ }
+
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(*I)) {
const FunctionDecl *DiagD;
if (!FD->hasBody(DiagD))
DiagD = FD;
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp (revision 204559)
+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -1216,6 +1216,9 @@
if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
if (MD->isVirtual() || IsDisallowedCopyOrAssign(MD))
return false;
+
+ if (MD->getAccess() == AS_private)
+ return true;
} else {
// 'static inline' functions are defined in headers; don't warn.
if (FD->isInlineSpecified() &&
@@ -1248,22 +1251,7 @@
}
void Sema::MarkUnusedFileScopedDecl(const DeclaratorDecl *D) {
- if (!D)
- return;
-
- if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
- const FunctionDecl *First = FD->getFirstDecl();
- if (FD != First && ShouldWarnIfUnusedFileScopedDecl(First))
- return; // First should already be in the vector.
- }
-
- if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
- const VarDecl *First = VD->getFirstDecl();
- if (VD != First && ShouldWarnIfUnusedFileScopedDecl(First))
- return; // First should already be in the vector.
- }
-
- if (ShouldWarnIfUnusedFileScopedDecl(D))
+ if (D && ShouldWarnIfUnusedFileScopedDecl(D))
UnusedFileScopedDecls.push_back(D);
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
