Let me add to the plate a simple proposal though still a bit verbose and
probably not that different from a manual cast
Defining a template method in SimObject:
template <class Obj>
Obj params()
{
return static_cast<Obj>(_params);
Or more secure:
param = dynamic_cast<Obj>(_params);
assert(param);
return param;
}
And delegate the type specification on the client side:
For example in my shiny new
Class MyObject : public SimObject {}
I would use
auto p = params<MyObjectParams*>();
You still have to type your param type so that doesn’t make it the best but I
believe it’s the simplest one
Giacomo
From: Jason Lowe-Power via gem5-dev <[email protected]>
Sent: 22 October 2020 16:26
To: gem5 Developer List <[email protected]>
Cc: Gabe Black <[email protected]>; Jason Lowe-Power <[email protected]>
Subject: [gem5-dev] Re: SimObject params() method
Hey Gabe,
Thanks for bringing this up. I have also been bothered by the lack of
consistency with how params are used. I can't think of an example of when you
need to store the params object. I would be all for getting rid of the params()
function and updating the documentation to say that it's best practice to *not*
save the params struct after the constructor. If some object had a good reason
to go against this best practice, that would be OK, and we wouldn't need to
enforce any specific design or pattern on these exceptions. I would prefer to
remove the params() function than add more template magic.
On Thu, Oct 22, 2020 at 1:18 AM Gabe Black via gem5-dev
<[email protected]<mailto:[email protected]>> wrote:
Hi folks. I'm looking at the SimObject header, and I see a few things in there
which are marked as part of the API and maybe shouldn't be. Specifically I'm
talking about the Params typedef, and the params() method. There is also the
_params member variable which I can see being a part of the API since it can be
used by other classes to make their own params() function (more on that below),
but the Params typedef is more or less an implementation detail, and the
params() method is essentially worthless because it returns a SimObjectParams
which doesn't have anything except the object's name in it, and you can already
get that with the name() method.
I agree. I think the typedef is useless and shouldn't be in the API. It's
unfortunate that the API proposals didn't get more reviews. I think it's
probably OK to just drop that from the API, but the params() function itself is
going to need to be deprecated.
Params pattern
This gets to the whole Params params() pattern which is sporadically
implemented in some SimObjects in gem5. This pattern is that a given SimObject
subclass will define a Params typedef which corresponds to its Params struct
type, and then also define a params() method which returns the _params from
SimObject cast up to that type.
The Params typedef itself primarily makes the definition of the constructor a
little more legible, since the FullFooTypeForTheArmISAParams can be really
verbose.
I think verbose is fine. I would vote to abolish all params typedefs.
Storing the params struct itself could theoretically serve the purpose of
having a bunch of parameters and not having to create a member variable for
each one, spend a bunch of text copying values over in the constructor, etc. I
think most of the time this is unnecessary, but if an object has tons of values
in it for some reason this could make sense.
I don't think there are any examples of this in the codebase. I think in all
cases the params data is copied into member variables. If there are cases where
data isn't copied, I doubt it was with a strong reason in mind. The one
exception to this might be Ruby, but it's an exception for all the wrong
reasons ;).
The params() method then makes the _params member available with the
appropriate type, so that all the FooParams members are accessible. It also
makes the params struct accessible outside the object which is used in a place
or two to read parameters without there needing to be a member in the object,
and probably some sort of accessor to read it.
There are two main problems with this system. First, when used, it adds a small
but not trivial amount of boilerplate to any given class. Second, it's very
sporadically implemented. I think in a lot of places it's there just because
people aren't sure what it's for or if they need it, so they just put one there
regardless. I think I've done that in the past.
Alternative
The existence of the Params type and the params() method could be partially
eliminated by defining a templated params() method which took a SimObject
reference and/or pointer as its first parameter. It could then figure out what
Params struct went with that SimObject type using typedef/template magic set up
by the Params building code, and then perform the appropriate cast.
This has three downsides, two minor and one more serious. The minor one is that
when a class uses this method internally, it would have to do something like
params(this) instead of just params(). That's a fairly minor difference and not
that big a deal. For external consumers that would be less of a problem since
it would change from foo->params() to params(foo).
The second minor issue is that the name params() is very short, and likely to
collide with other names. We could define that with SimObject as a static
method, but then that would make foo->params() turn into the more verbose
SimObject::params(foo), or (and I haven't checked if this is legal syntax) the
more odd looking foo->params(foo). The params() class can't be a non-static
method, because then the type of "this" would be fixed by where it was defined,
meaning it would not cast _params to the right type. I was not able to find any
mechanism in c++ that would let you treat "this" as an argument for template
type resolution.
The third more serious problem is that this implicitly breaks the ability to
use two different SimObject types in python to represent the same SimObject
type in C++. I don't know if this happens in practice, and it's also broken by
the original Params pattern, since there can be only one typedef in a given
class. Since Params is applied adhoc manually, something that is generally not
good, it actually avoids this problem by just not existing anywhere that would
break that assumption.
Other option
Another option would be to have a templated class which would define a Params
type and a params() method, and inherit that into each SimObject which wants to
have those members. It would itself inherit from its parameter which would keep
the inheritance hierarchy intact, and make it possible to override Params and
params from super classes:
FooObject : public ParamsMembers<SimObject>
This has a similar problem to the above if exactly what Params type to use is
automatic, although here it could be an additional template argument. This also
trades some boilerplate for less boilerplate, has to be applied manually to any
classes that want to take advantage of it, and obfuscates the definition of
those classes.
Gabe
_______________________________________________
gem5-dev mailing list -- [email protected]<mailto:[email protected]>
To unsubscribe send an email to
[email protected]<mailto:[email protected]>
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
