> On Oct. 1, 2015, 10:11 p.m., Steve Reinhardt wrote: > > Seems like a good idea, but I'm curious about the decision to use the extra > > defaulted parameter. From a usability issue, it might be better just to > > introduce a new schedRelBreak() function that just calls schedBreak(n + > > curTick()). > > Curtis Dunham wrote: > I don't disagree. The original patch was just that and internal > commentary led to this version. They both have a little bit of unsavoryness, > but I'm certainly okay prioritizing the usability aspect. > > Steve Reinhardt wrote: > Actually neither one seems particularly unsavory to me. However, I've > never really liked having boolean flag parameters like this in C++ since > there's no explanation at the call site about what they're doing (unless the > caller is thoughtful enough to put in a comment)... unlike in python where > you can get some documentation by using keyword params. It seems like a > worthwhile tradeoff when it enables you to reuse a large complex function by > allowing some minor detail to be tweaked by the caller though. In this case, > though, where we're only talking about a one-line function anyway, and we can > get the same level of code commonality by having schedRelBreak() just call > schedBreak(), I don't really see any advantage to this current version.
Updated with the function named as suggested. - Curtis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3142/#review7327 ----------------------------------------------------------- On Oct. 5, 2015, 10:50 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3142/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2015, 10:50 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Allow schedBreak() to set a breakpoint by relative rather than absolute tick. > > > Diffs > ----- > > src/sim/debug.hh bc07f37641295bfb302950805393872d00ce68ee > src/sim/debug.cc bc07f37641295bfb302950805393872d00ce68ee > > Diff: http://reviews.gem5.org/r/3142/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
