[cfe-commits] Clang change to detect and warn about unused private members

[Daniel Jasper]

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
> insert+make_pair.
>
> - 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 (
http://clang.llvm.org/doxygen/DeclFriend_8h_source.html#l00041), which
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!
>
> Jordan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120604/0b88626f/attachment.html>