Changed method name and comment and committed as r158395. On Wed, Jun 13, 2012 at 8:02 AM, Richard Smith <[email protected]>wrote:
> On Tue, Jun 12, 2012 at 10:57 PM, James Dennett <[email protected]>wrote: > >> 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). > > > OK, shouldVisitImplicitCode sounds reasonable. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
