On Fri, Apr 18, 2014 at 5:40 AM, Nuno Lopes <[email protected]> wrote:
> Hi, > > Please find in attach a second version of the patch, which reuses the > cache of defined records properly. > With this patch, -Wunused-member-function now flags unused private methods > whenever the class is fully defined. > This patch does not attempt to fix false positives that were being > triggered before in classes in anonymous namespaces. I'll fix those > afterwards. > > OK to commit? > No, it's not OK to delete the early pruning of UnusedFileScopedDecls. Without that, we'll load in all unused declarations within a module for every compilation that includes the module. The approach taken by warning and the existing unused private field warning interact badly with modules in general -- they force us to deserialize most private members within every imported module, just in case the class became completely-defined within this compilation, because their approach is "for each maybe-unused thing, check if it's unused, then diagnose". Instead of the current approach, we could go for "for each class completed in this compilation, check for unused private members". > Thanks, > Nuno > > ----- Original Message ----- From: Nuno Lopes > Sent: Sunday, March 23, 2014 11:14 PM > Subject: Improving -Wunused-member-function > > > 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). >> >> 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. >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
