Giacomo Travaglini has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/37775 )

Change subject: scons, python: Remove SmartDict from python utilities
......................................................................

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 <giacomo.travagl...@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37775
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Reviewed-by: Hoa Nguyen <hoangu...@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbr...@ucdavis.edu>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/SConscript
M src/python/SConscript
M src/python/m5/util/__init__.py
D src/python/m5/util/smartdict.py
4 files changed, 2 insertions(+), 161 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Hoa Nguyen: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/SConscript b/src/SConscript
index 9446242..b55f485 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -824,7 +824,7 @@
             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 @@
 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 50f467e..6611e6a 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -53,7 +53,6 @@
 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 d26bf4e..b319679 100644
--- a/src/python/m5/util/__init__.py
+++ b/src/python/m5/util/__init__.py
@@ -52,7 +52,6 @@
 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 addb0c5..0000000
--- 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' ]

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/37775
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I960fbfb1ec0f703e1e372dd752ee75f00632acac
Gerrit-Change-Number: 37775
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Hoa Nguyen <hoangu...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to