Make Spec construction simpler (#5227)

lib/spack/spack/spec.py

@@ -1133,34 +1133,19 @@ def __init__(self, spec_like, **kwargs):
         if not isinstance(spec_like, string_types):
             raise TypeError("Can't make spec out of %s" % type(spec_like))
 
-        spec_list = SpecParser().parse(spec_like)
+        # parse string types *into* this spec
+        spec_list = SpecParser(self).parse(spec_like)
         if len(spec_list) > 1:
             raise ValueError("More than one spec in string: " + spec_like)
         if len(spec_list) < 1:
             raise ValueError("String contains no specs: " + spec_like)
 
-        # Take all the attributes from the first parsed spec without copying.
+        # Specs are by default not assumed to be normal, but in some
-        # This is safe b/c we throw out the parsed spec.  It's a bit nasty,
+        # cases we've read them from a file want to assume normal.
-        # but it's nastier to implement the constructor so that the parser
+        # This allows us to manipulate specs that Spack doesn't have
-        # writes directly into this Spec object.
+        # package.py files for.
-        other = spec_list[0]
+        self._normal = kwargs.get('normal', False)
-        self.name = other.name
+        self._concrete = kwargs.get('concrete', False)
-        self.versions = other.versions
-        self.architecture = other.architecture
-        self.compiler = other.compiler
-        self.compiler_flags = other.compiler_flags
-        self.compiler_flags.spec = self
-        self._dependencies = other._dependencies
-        self._dependents = other._dependents
-        self.variants = other.variants
-        self.variants.spec = self
-        self.namespace = other.namespace
-        self._hash = other._hash
-        self._cmp_key_cache = other._cmp_key_cache
-
-        # Specs are by default not assumed to be normal or concrete.
-        self._normal = False
-        self._concrete = False
 
         # Allow a spec to be constructed with an external path.
         self.external_path = kwargs.get('external_path', None)
@@ -3131,9 +3116,17 @@ def __init__(self):
 
 class SpecParser(spack.parse.Parser):
 
-    def __init__(self):
+    def __init__(self, initial_spec=None):
+        """Construct a new SpecParser.
+
+        Args:
+            initial_spec (Spec, optional): provide a Spec that we'll parse
+                directly into. This is used to avoid construction of a
+                superfluous Spec object in the Spec constructor.
+        """
         super(SpecParser, self).__init__(_lexer)
         self.previous = None
+        self._initial = initial_spec
 
     def do_parse(self):
         specs = []
@@ -3257,8 +3250,14 @@ def spec(self, name):
             spec_namespace = None
             spec_name = None
 
-        # This will init the spec without calling __init__.
+        if self._initial is None:
-        spec = Spec.__new__(Spec)
+            # This will init the spec without calling Spec.__init__
+            spec = Spec.__new__(Spec)
+        else:
+            # this is used by Spec.__init__
+            spec = self._initial
+            self._initial = None
+
         spec.name = spec_name
         spec.versions = VersionList()
         spec.variants = VariantMap(spec)
@@ -3316,7 +3315,7 @@ def spec(self, name):
                 # Get spec by hash and confirm it matches what we already have
                 hash_spec = self.spec_by_hash()
                 if hash_spec.satisfies(spec):
-                    spec = hash_spec
+                    spec._dup(hash_spec)
                     break
                 else:
                     raise InvalidHashError(spec, hash_spec.dag_hash())