----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/882/#review1592 -----------------------------------------------------------
While this might looks somewhat like the CRTP, it's a pretty wierd version of it, such that I probably wouldn't even call it that. The fact that you don't even use the template parameter is pretty odd. As far as I can tell, you added the template so you could avoid having to declare the various static members for each version of the class. I'd personally think it's pretty confusing. Wouldn't it be simpler to get rid of just about all of this code, including the accessors and just have a single public member that is a "static const FaultVals vals"? For each fault? If you really don't want to do this, I won't stand in your way (because I do believe done > perfect), but it took a long time for me to figure out what the heck was going on. - Nathan On 2011-09-25 03:04:11, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/882/ > ----------------------------------------------------------- > > (Updated 2011-09-25 03:04:11) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Alpha: Use the CRTP to simplify Alpha's fault classes. > > > Diffs > ----- > > src/arch/alpha/ev5.cc e21224136182 > src/arch/alpha/faults.hh e21224136182 > src/arch/alpha/faults.cc e21224136182 > > Diff: http://reviews.m5sim.org/r/882/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
