First off, what do people think about turning this on by default, or at least
making it part of -Wunused? Daniel's being conservative by leaving it off, but
you know what people say about off-by-default warnings...
---
As for the patch…sorry, one last problem.
+ // Mark as fully defined for now to stop recursion in case of mutual
+ // friends.
+ FullyDefined.insert(RD);
I thought about this some more, and it seems like this will cause incorrect
warnings:
class B;
class A {
int x;
public:
friend class B;
doSomethingWithX();
}
class B {
int y;
public:
friend class A;
}
When looking through A's decls (because of A::x), we'll come across "friend
class B". We'll look through B's decls and find "friend class A". A has already
been marked complete, so we mark B complete. Then we go back to A and find
"doSomethingWithX()", and A is now (correctly and finally) marked incomplete.
When we get to B::y, though, we're in trouble, because B has already been
marked as complete via A!
The usual thing to do here is add an "in progress" set, but I'm still not sure
exactly how to use that for warnings. I think the best thing to do (and
possibly the most correct thing to do) is to simply give up when we see mutual
friends declarations -- that is, tentatively mark classes as not fully defined
instead of fully defined. That gives us a conservative set of warnings, even if
we miss a few valid cases.
> Finally, even though there's not a big hit traversing the fields at the end,
> we probably still shouldn't pay for this if the diagnostic is off. Can you
> check for that before inserting into UnusedPrivateFields, or even doing all
> the tests?
>
> I am now checking whether the diagnostic is enabled before inserting into the
> set and before doing the checks at the end of the translation unit. Checking
> before removing from the (then empty) set probably does not give any benefit.
> I am not sure whether the check on the initializer arguments is expensive
> (especially the call to HasSideEffects). What do you think?
Let's leave it as is. Looking up a diagnostic isn't /that/ cheap, and most
initializers are something simple like a VarDecl, a constant, or a
FunctionCall.
I think once you've fixed the friend problem you can go ahead and commit.
Thanks for working on this!
Jordan_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
