Use of params() predates me, but I think it's just so that you don't need a bunch of boilerplate to store all the parameters local to the object. It is handy that way, although it makes it a little harder to tell what all the properties are and where they're coming from.
Gabe On Wed, Jan 8, 2020 at 12:06 PM Daniel Carvalho <[email protected]> wrote: > Hello Gabe, > > 1. Const. > > 2. References. > 3. I agree; however, in a higher level, what is the purpose of params()? > Most of the classes I have seen simply duplicate the parameters as member > variables, and in a way it serves to "expose" them so that the user becomes > aware of their existence. Is keeping them hidden behind the params() call a > decision we want to concede? > > > Regards, > Daniel > Em terça-feira, 7 de janeiro de 2020 17:10:42 GMT+1, Jason Lowe-Power < > [email protected]> escreveu: > > Hey Gabe, > > Real quick reply as I'm not sure how much time I'll have to dig into this > in the next couple of days. > > 1. I agree it should be const everywhere. > 2. I agree it should be references everywhere. > 3. I'll have to think about this more :). > > If you're planning on working on 1 & 2 or you would like some help with > these, please create a Jira issue so we can track it. > > Thanks, > Jason > > On Mon, Jan 6, 2020 at 6:04 PM Gabe Black <[email protected]> wrote: > > > Hi folks. Right now we have a mishmash of ways to store the params > > structure passed to a SimObject class, if that class actually does store > > the params in the first place. > > > > The base SimObject class itself stores a const pointer to the very base > > SimObjectParams type. It defines a params() method which returns it as a > > const pointer. > > > > Some other classes (I only see the base System class atm, although there > > may be others) declare their own pointer locally. In the case of System, > > this pointer is not const, but the function that returns it makes it > const > > (and casts it to const before returning it for some reason?). > > > > Other classes still declare a shadow _params member which is a const &, > and > > return that from their params() method. > > > > Then beyond that, some types have constructors which expect a pointer to > > the params struct, and others expect a reference. > > > > This can result in a lot of confusion and annoying compile errors where > you > > have to go in and add &s and *s in random places to get to the type you > > want. We should pick something and standardize on it. > > > > > > 1. Const vs. non-const. My vote is const. The params structs are set up > in > > python, and the are often not looked at again once SimObjects are > > instantiated, although they may be. Allowing them to be fiddled with > > introduces the possibility of their value being stale and not reflected > in > > the actual behavior of the simulation. These structures should be a one > way > > uninterrupted conduit for config values from python to C++, passed > through > > exactly once. > > > > If the params are treated as const (and once that's consistently > applied), > > then the create() method can be marked const which will enforce that > going > > forward. > > > > > > 2. Pointer vs. reference. Pointers were the historic implementation, and > > mixing in references inconsistently was a bad idea IMHO, but since the > > params structure should always be there and should always be one > particular > > thing, I think references are the superior option. I think that means we > > should go through and convert all the existing SimObjects to accept > > references in their constructors, SimObject to store a const reference, > and > > params() to return a const reference. > > > > > > 3. One off params() definition vs. centralized definition. > > > > As of today, many (but not all) SimObject classes define a params() > method > > which casts the _param member to the param type used by that class to > > return. That means all those classes have individual typedefs which set > up > > a Params type for that class, and a params() function which is often (but > > not always) just a copy of the n other params() functions floating > around. > > > > It would be nice to centralize this somehow. I don't have a great drop in > > replacement for this since that just doesn't seem to be compatible with > the > > way C++ works as of today, but I think it could be simplified a bit with > a > > couple definitions the SimObject python<->C++ bridge could generate. > These > > are rough ideas, so suggestions for improvements are welcome. > > > > template <typename SimObjectType> > > struct ParamsForSimObjectType; > > > > template <typename SimObjectType> > > using Params = ParamsForSimObjectType<SimObjectType>::Type; > > > > (generated by scons) > > template <> > > struct ParamsForSimObjectType<YourFavoriteSimObject> > > { > > using Type = const YourFavoriteSimObjectParams &; > > }; > > > > Then if you need a Params type for a SimObject type, you can use > > Params<System> for instance. > > > > template <typename SimObjectType> > > Params<SimObjectType> > > params(SimObjectType *so) > > { > > try { > > return dynamic_cast<Params<SimObjectType>>(so->base_params()); > > } catch (std::bad_cast &exc) { > > panic("Failed to cast to derived param type."); > > } > > } > > > > There are two related downsides to this version of params. First, this > > shouldn't live out in the global namespace since it's so generic. We'd > want > > to put it in some sort of namespace, or make it a static member of > > something like the SimObject class. If it's part of a class, it might be > > confusing to people who are used to params() being a method and not a > > static function since it would now need a pointer to the simobject. > > > > This would mean that, for instance, a constructor of a class Foo could > just > > use params(this) to get a properly cast FooParams without having to have > a > > bunch of boilerplate and plumbing for every class. > > > > If it's used a lot, you could set up a temporary like this to make things > > shorter: > > auto p = params(this); > > > > It's possible that the template-ey-ness of this could get annoying with > > base classes of SimObjects which aren't set up in python, potentially > > requiring casting down to the class pybind set up to get the right > answer. > > I think in that case you could define params() through pybind as well, > and > > rely on function overload resolution and not templating to get the right > > implementation. > > > > > > I'll likely put this together into a design doc at least to have a > record, > > but since it isn't totally baked and is relatively simple I thought I'd > > throw it out there as an email first. > > > > Gabe > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
