Hi Nico,
Thanks for trying out the patch and for sending in the test cases.
I hope I'll be able to devote some time to this again next week to fix the
problems you've found.
Thanks,
Nuno
----- Original Message -----
From: "Nico Weber" <[email protected]>
Sent: Wednesday, March 26, 2014 9:41 PM
Subject: Re: Improving -Wunused-member-function
Hi Nuno,
this looks like a cool warning. I tried it on chromium; it looks like the
implementation could use some polishing.
* We have a DISALLOW_COPY_AND_ASSIGN macro that expands to a constructor
name, to be places in a "private:" section to make a class not
instantiatable (
https://code.google.com/p/chromium/codesearch#chromium/src/base/macros.h&l=45).
The warning complains about these constructors. Maybe it shouldn't
warn
on declared-but-not-defined constructors / destructors / assignment
operators?
* It complains about private functions that override pure virtual methods –
but removing them leads to "can't instantiate class with pure virtual
methods" errors.
Nico
On Sun, Mar 23, 2014 at 3:14 PM, Nuno Lopes <[email protected]> wrote:
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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
