On Jan 15, 2010, at 1:38 PM, David Greene wrote:> On Friday 15 January 2010 15:09, Dan Gohman wrote: > >>> Sound good? reimplement >> >> Unlimited-recursion dumping is what the existing dump routines >> already do, so it's a little odd to have a flag to allow these > > Which existing dump routines are you referring to?dumpr(). I guess it wasn't commented. It is now :-).> >> new dump routines to do the same thing. I guess you could >> refactor the old ones to call the new ones and eliminate some >> redundant code, if you wanted. > > We probably should.Ok. Dan
On Friday 15 January 2010 16:23, Dan Gohman wrote:> > Which existing dump routines are you referring to? > > dumpr(). I guess it wasn't commented. It is now :-). > > >> new dump routines to do the same thing. I guess you could > >> refactor the old ones to call the new ones and eliminate some > >> redundant code, if you wanted. > > > > We probably should. > > Ok.All right, I'll work on that. -Dave
On Friday 15 January 2010 16:23, Dan Gohman wrote:> On Jan 15, 2010, at 1:38 PM, David Greene wrote: > > On Friday 15 January 2010 15:09, Dan Gohman wrote: > >>> Sound good? reimplement > >> > >> Unlimited-recursion dumping is what the existing dump routines > >> already do, so it's a little odd to have a flag to allow these > > > > Which existing dump routines are you referring to? > > dumpr(). I guess it wasn't commented. It is now :-).Ah, one thing. dumpr uses DumpNodesr which does the "once" thing. I actually would prefer a full dump. Perhaps we shouldn't try to unify them, or maybe provide a flag to control behavior. In the past separate APIs have been preferred over flags. I can rename the ones I added to be more consistent with the existing stuff. Opinions? -Dave
On Jan 15, 2010, at 2:34 PM, David Greene wrote:> On Friday 15 January 2010 16:23, Dan Gohman wrote: >> On Jan 15, 2010, at 1:38 PM, David Greene wrote: >>> On Friday 15 January 2010 15:09, Dan Gohman wrote: >>>>> Sound good? reimplement >>>> >>>> Unlimited-recursion dumping is what the existing dump routines >>>> already do, so it's a little odd to have a flag to allow these >>> >>> Which existing dump routines are you referring to? >> >> dumpr(). I guess it wasn't commented. It is now :-). > > Ah, one thing. dumpr uses DumpNodesr which does the "once" thing. > I actually would prefer a full dump. Perhaps we shouldn't try to > unify them, or maybe provide a flag to control behavior. In the > past separate APIs have been preferred over flags. I can rename > the ones I added to be more consistent with the existing stuff. > > Opinions?I use the GraphViz viewer almost exclusively, so I don't have a strong opinion. Methods with lots of flags are inconvenient to call from a debugger. I'd suggesting coming up with a few common use cases, and providing interfaces to cover those use cases, and not trying to provide lots of extra generality. If SDNode::dumpr() had built-in cycle detection, and indicated cycles with big capital letters, would you still want a recursive dump which doesn't do the "once" thing? Or, if the "once" thing had a more human-oriented syntax, would it be usable? Dan