> On Oct. 1, 2015, 3: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.
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. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3142/#review7327 ----------------------------------------------------------- On Sept. 30, 2015, 2:42 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3142/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2015, 2:42 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 11da0268127783772981a52485b5b883b1607c8b > src/sim/debug.cc 11da0268127783772981a52485b5b883b1607c8b > > 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
