From 56494ed699ac66a9f1bb5e818f06dd0fd3b51497 Mon Sep 17 00:00:00 2001 From: Jason Lowe-Power Date: Wed, 13 Oct 2021 11:06:36 -0700 Subject: [PATCH] python: Add check to SimObject for __init__ When extending a SimObject by subclassing, if you don't call `super().__init__()` you get a confusing infinite recursion error. The infinite recursion occurs because SimObject overrides `__getattr__`. So, if an attribute is accessed that is set in SimObject.__init__ but that function hasn't been called there's a problem. This patch adds another member variable to track if __init__ has been called. This member variable is set to False in the *meta class* so that it will always be available, even if __init__ has not been called. There is one check for whether init has been called in the __getattr__ function. This is where I have experienced prior issues. This function could be called from other SimObject functions, if needed. With this change, a helpful error is shown telling the user to be sure to call super().__init__ in the specific class that is missing the call. Note: I have been bitten by this an embarrassing number of times. A helpful error message would have saved me many hours. Change-Id: Id919c540b23fc2783e203ef625bce3000ba808a9 Signed-off-by: Jason Lowe-Power Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51568 Maintainer: Jason Lowe-Power Maintainer: Andreas Sandberg Reviewed-by: Andreas Sandberg Tested-by: kokoro --- src/python/m5/SimObject.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py index dec630c1ae..bf5ab54571 100644 --- a/src/python/m5/SimObject.py +++ b/src/python/m5/SimObject.py @@ -479,6 +479,7 @@ class MetaSimObject(type): cls._children = multidict() # SimObject children cls._port_refs = multidict() # port ref objects cls._instantiated = False # really instantiated, cloned, or subclassed + cls._init_called = False # Used to check if __init__ overridden # We don't support multiple inheritance of sim objects. If you want # to, you must fix multidict to deal with it properly. Non sim-objects @@ -1323,6 +1324,7 @@ class SimObject(object, metaclass=MetaSimObject): self._ccObject = None # pointer to C++ object self._ccParams = None self._instantiated = False # really "cloned" + self._init_called = True # Checked so subclasses don't forget __init__ # Clone children specified at class level. No need for a # multidict here since we will be cloning everything. @@ -1352,6 +1354,14 @@ class SimObject(object, metaclass=MetaSimObject): for key,val in kwargs.items(): setattr(self, key, val) + def _check_init(self): + """Utility function to check to make sure that all subclasses call + __init__ + """ + if not self._init_called: + raise RuntimeError(f"{str(self.__class__)} is missing a call " + "to super().__init__()") + # "Clone" the current instance by creating another instance of # this instance's class, but that inherits its parameter values # and port mappings from the current instance. If we're in a @@ -1385,6 +1395,11 @@ class SimObject(object, metaclass=MetaSimObject): return ref def __getattr__(self, attr): + # Check for infinite recursion. If this SimObject hasn't been + # initialized with SimObject.__init__ this function will experience an + # infinite recursion checking for attributes that don't exist. + self._check_init() + if attr in self._deprecated_params: dep_param = self._deprecated_params[attr] dep_param.printWarning(self._name, self.__class__.__name__)