On Tue, Jun 12, 2012 at 10:48 PM, Richard Smith <[email protected]> wrote: > Hi Daniel, > > > On Tue, Jun 12, 2012 at 10:36 PM, Daniel Jasper <[email protected]> wrote: >> >> Please find the attached patch, which changes the RecursiveASTVisitor >> behavior for CXXForRangeStmts, for review. >> >> Depending on whether a RecursiveASTVisitor should visit implicit >> declarations, we need to do different things when visiting a range-based for >> loop. >> >> If visiting implicit declarations, we can simply visit all children as we >> currently do. >> >> If not, we should explicitly visit the loop variable (but not its implicit >> initialization) and the range initialization. Otherwise, we would not visit >> the range initialization at all (although it is explicit) because it is part >> of an implicit VarDecl. > > > The patch looks great, but please update the comment for > shouldVisitImplicitDeclarations to indicate that it also controls whether we > visit implicit expressions in some cases. (I'm wary of renaming it, because > that might silently break external RAV users...)
While breakage is possible (and worthy of caution), shouldVisitImplicitDeclarations() was added just a week ago. As one of its users, I'd happily pay the price of updating my code if it meant avoiding a misleading function name. (One of those eternal tradeoffs: the more specific a name is, the more resistant it is to future changes.) That said, I don't have a good suggestion for a name; shouldVisitImplicitCode, maybe. By the time we cover declarations and expressions, there's not a whole lot left (well, other statements, I guess, but why make the name so specific that it rules that out). -- James _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
