From d57438108119e8e455957487715ec8d6d07ee6f2 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Thu, 19 Nov 2020 11:53:09 +0000 Subject: [PATCH] scons, python: Remove SmartDict from python utilities The SmartDict, used by buildEnv, has been added long time ago for the following reasons: (checking its documentation) --- The SmartDict class fixes a couple of issues with using the content of os.environ or similar dicts of strings as Python variables: 1) Undefined variables should return False rather than raising KeyError. 2) String values of 'False', '0', etc., should evaluate to False (not just the empty string). --- These are valid reasons, but I believe they should be addressed in a more standardized way by using a common dictionary. 1) We should simply rely on dict.get if buildEnv.get('KEY', False/None): 2) We should discourage the use of stringified False or 0. If we are using a dictionary, can't we just pass those values as booleans? The SmartDict is basically converting every value into a string ("Variable") at every access (__getitem__) The Variable is a string + some "basic" conversion methods What is the problem of passing every dict value as a string? The problem is the ambiguity on the boolean conversion. If a variable is modelling a boolean, we can return true if the value is 'yes', 'true'... and false if the value is 'no', 'false' etc. We should raise an exception if it is something different, like a typo (e.g.) 'Fasle'. But if the variable is not modelling a boolean, we don't know how to handle that. How should we convert 'mystring' ? If we decide to treat 'mystring' as True (which is basically what a str.__bool__ would return) we will break typoes detection, as 'Fasle' will now be converted to True, rather than raising an exception. Change-Id: I960fbfb1ec0f703e1e372dd752ee75f00632acac Signed-off-by: Giacomo Travaglini Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37775 Reviewed-by: Jason Lowe-Power Reviewed-by: Hoa Nguyen Maintainer: Bobby R. Bruce Maintainer: Gabe Black Tested-by: kokoro --- src/SConscript | 4 +- src/python/SConscript | 1 - src/python/m5/util/__init__.py | 1 - src/python/m5/util/smartdict.py | 157 -------------------------------- 4 files changed, 2 insertions(+), 161 deletions(-) delete mode 100644 src/python/m5/util/smartdict.py diff --git a/src/SConscript b/src/SConscript index 9446242c4..b55f48532 100644 --- a/src/SConscript +++ b/src/SConscript @@ -824,7 +824,7 @@ class DictImporter(object): return mod if fullname == 'm5.defines': - mod.__dict__['buildEnv'] = m5.util.SmartDict(build_env) + mod.__dict__['buildEnv'] = dict(build_env) return mod source = self.modules[fullname] @@ -894,7 +894,7 @@ def makeDefinesPyFile(target, source, env): import _m5.core import m5.util -buildEnv = m5.util.SmartDict($build_env) +buildEnv = dict($build_env) compileDate = _m5.core.compileDate gem5Version = _m5.core.gem5Version diff --git a/src/python/SConscript b/src/python/SConscript index 50f467ead..6611e6a80 100644 --- a/src/python/SConscript +++ b/src/python/SConscript @@ -53,7 +53,6 @@ PySource('m5.util', 'm5/util/dot_writer_ruby.py') PySource('m5.util', 'm5/util/grammar.py') PySource('m5.util', 'm5/util/jobfile.py') PySource('m5.util', 'm5/util/multidict.py') -PySource('m5.util', 'm5/util/smartdict.py') PySource('m5.util', 'm5/util/sorteddict.py') PySource('m5.util', 'm5/util/terminal.py') PySource('m5.util', 'm5/util/pybind.py') diff --git a/src/python/m5/util/__init__.py b/src/python/m5/util/__init__.py index d26bf4e60..b319679ed 100644 --- a/src/python/m5/util/__init__.py +++ b/src/python/m5/util/__init__.py @@ -52,7 +52,6 @@ from . import jobfile from .attrdict import attrdict, multiattrdict, optiondict from .code_formatter import code_formatter from .multidict import multidict -from .smartdict import SmartDict from .sorteddict import SortedDict # panic() should be called when something happens that should never diff --git a/src/python/m5/util/smartdict.py b/src/python/m5/util/smartdict.py deleted file mode 100644 index addb0c5f5..000000000 --- a/src/python/m5/util/smartdict.py +++ /dev/null @@ -1,157 +0,0 @@ -# Copyright (c) 2005 The Regents of The University of Michigan -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer; -# redistributions in binary form must reproduce the above copyright -# notice, this list of conditions and the following disclaimer in the -# documentation and/or other materials provided with the distribution; -# neither the name of the copyright holders nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -# The SmartDict class fixes a couple of issues with using the content -# of os.environ or similar dicts of strings as Python variables: -# -# 1) Undefined variables should return False rather than raising KeyError. -# -# 2) String values of 'False', '0', etc., should evaluate to False -# (not just the empty string). -# -# #1 is solved by overriding __getitem__, and #2 is solved by using a -# proxy class for values and overriding __nonzero__ on the proxy. -# Everything else is just to (a) make proxies behave like normal -# values otherwise, (b) make sure any dict operation returns a proxy -# rather than a normal value, and (c) coerce values written to the -# dict to be strings. - -from __future__ import print_function -from __future__ import absolute_import -import six -if six.PY3: - long = int - -from .convert import * -from .attrdict import attrdict - -class Variable(str): - """Intelligent proxy class for SmartDict. Variable will use the - various convert functions to attempt to convert values to useable - types""" - def __int__(self): - return toInteger(str(self)) - def __long__(self): - return toLong(str(self)) - def __float__(self): - return toFloat(str(self)) - def __bool__(self): - return toBool(str(self)) - # Python 2.7 uses __nonzero__ instead of __bool__ - __nonzero__ = __bool__ - def convert(self, other): - t = type(other) - if t == bool: - return bool(self) - if t == int: - return int(self) - if t == long: - return long(self) - if t == float: - return float(self) - return str(self) - def __lt__(self, other): - return self.convert(other) < other - def __le__(self, other): - return self.convert(other) <= other - def __eq__(self, other): - return self.convert(other) == other - def __ne__(self, other): - return self.convert(other) != other - def __gt__(self, other): - return self.convert(other) > other - def __ge__(self, other): - return self.convert(other) >= other - - def __add__(self, other): - return self.convert(other) + other - def __sub__(self, other): - return self.convert(other) - other - def __mul__(self, other): - return self.convert(other) * other - def __div__(self, other): - return self.convert(other) / other - def __truediv__(self, other): - return self.convert(other) / other - - def __radd__(self, other): - return other + self.convert(other) - def __rsub__(self, other): - return other - self.convert(other) - def __rmul__(self, other): - return other * self.convert(other) - def __rdiv__(self, other): - return other / self.convert(other) - def __rtruediv__(self, other): - return other / self.convert(other) - -class UndefinedVariable(object): - """Placeholder class to represent undefined variables. Will - generally cause an exception whenever it is used, but evaluates to - zero for boolean truth testing such as in an if statement""" - def __bool__(self): - return False - - # Python 2.7 uses __nonzero__ instead of __bool__ - __nonzero__ = __bool__ - -class SmartDict(attrdict): - """Dictionary class that holds strings, but intelligently converts - those strings to other types depending on their usage""" - - def __getitem__(self, key): - """returns a Variable proxy if the values exists in the database and - returns an UndefinedVariable otherwise""" - - if key in self: - return Variable(dict.get(self, key)) - else: - # Note that this does *not* change the contents of the dict, - # so that even after we call env['foo'] we still get a - # meaningful answer from "'foo' in env" (which - # calls dict.__contains__, which we do not override). - return UndefinedVariable() - - def __setitem__(self, key, item): - """intercept the setting of any variable so that we always - store strings in the dict""" - dict.__setitem__(self, key, str(item)) - - def values(self): - for value in dict.values(self): - yield Variable(value) - - def items(self): - for key,value in dict.items(self): - yield key, Variable(value) - - def get(self, key, default='False'): - return Variable(dict.get(self, key, str(default))) - - def setdefault(self, key, default='False'): - return Variable(dict.setdefault(self, key, str(default))) - -__all__ = [ 'SmartDict' ] -- 2.30.2