On Mon, Jun 4, 2012 at 7:22 PM, Jordan Rose <[email protected]> wrote:
> > On Jun 4, 2012, at 1:43 , Daniel Jasper <[email protected]> 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
