[cfe-commits] Clang change to detect and warn about unused private members
djasper at google.com
Mon Jun 4 11:14:43 PDT 2012
On Mon, Jun 4, 2012 at 7:22 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> On Jun 4, 2012, at 1:43 , Daniel Jasper <djasper at google.com> wrote:
> > Thanks for coming up with this!! I propose a different solution which
> utilizes the fact that friend relationships are not transitive. Thus, if a
> class has a friend, it only needs to check whether all methods and nested
> classes of the friend are defined but does not need to consider the
> friend's friends.
> Oh, of course! Good catch.
> > Could you take one last look as the implementation of
> IsRecordFullyDefined has changed substantially?
> Thank you for being careful. :-) Only small comments this time, I think:
> - Please attach the & to the variable name for references, per LLVM style
> conventions (same as pointers).
> - You've got an isa followed by dyn_cast for CXXMethodDecl; please change
> to just dyn_cast (like CXXRecordDecl).
> - DenseMap has an operator, which is a little easier to read than
> - This is very nitpicky, but the style guide says to put all comments on
> their own line, and to end them with periods (full stops). So "// This is a
> template friend, give up" should be reformatted as such.
> Also, it took me a while to convince myself that CXXRecordDecls coming out
> of getFriendDecl() are also problematic, but what convinced me is that if
> they don't have a TypeSourceInfo, we can't possibly see their definition.
> It does seem like it is possible to have CXXRecordDecls come out of
> getFriendDecl(), though, and that might be worth commenting.
I have never managed to get a CXXRecordDecl out of getFriendDecl(). For
friend classes getFriendDecl() always seems to return 0. How do I create
such a case?
Also, it uses FriendUnion as internal data type (
hints at the result either being a NamedDecl for friend functions or a
TypeSourceInfo for friend classes. I have added comments and an else-branch
to the "if(..->getAsCXXRecordDecl())" so that if we don't understand the
FriendDecl, we mark the class as incomplete.
> I'm tempted to say that passing around a pair of maps to two different
> methods is worth a private class, but I guess it's fine the way it is.
> I think that's it. Good work!
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits