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
