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 <ja...@lowepower.com> 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 <gabebl...@google.com> 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 > gem5-dev@gem5.org > http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev