Repository: amaranth-lang/amaranth
Pull request #1360 (2024-05-18 00:56:02)

build.plat: allow spaces in environment variables on Windows
Specifically, allow spaces in:
- `set AMARANTH_ENV_<toolchain>=c:\program files\...`
- `set <tool>=c:\program files\...`

I don't actually understand *why* `SetLocal EnableDelayedExpansion` fixes this, but it seems to work. I found it on StackOverflow but I can't find the question anymore >.>


Patch:
diff --git a/amaranth/build/plat.py b/amaranth/build/plat.py
index 508c9ec..fade608 100644
--- a/amaranth/build/plat.py
+++ b/amaranth/build/plat.py
@@ -184,7 +184,8 @@ class TemplatedPlatform(Platform):
         "build_{{name}}.bat": """
             @rem {{autogenerated}}
             {{quiet("@echo off")}}
-            if defined {{platform._toolchain_env_var}} call %{{platform._toolchain_env_var}}%
+            SetLocal EnableDelayedExpansion
+            if defined {{platform._toolchain_env_var}} call "%{{platform._toolchain_env_var}}%"
             {{emit_commands("bat")}}
         """,
     }
@@ -283,8 +284,8 @@ class TemplatedPlatform(Platform):
                     template = ": ${{{env_var}:={name}}}"
                 elif syntax == "bat":
                     template = \
-                        "if [%{env_var}%] equ [\"\"] set {env_var}=\n" \
-                        "if [%{env_var}%] equ [] set {env_var}={name}"
+                        "if [!{env_var}!] equ [\"\"] set {env_var}=\n" \
+                        "if [!{env_var}!] equ [] set {env_var}={name}"
                 else:
                     assert False
                 commands.append(template.format(env_var=env_var, name=name))
@@ -308,7 +309,7 @@ class TemplatedPlatform(Platform):
             if context.parent["syntax"] == "sh":
                 return f"\"${env_var}\""
             elif context.parent["syntax"] == "bat":
-                return f"%{env_var}%"
+                return f"\"%{env_var}%\""
             else:
                 assert False
 
diff --git a/amaranth/hdl/__init__.py b/amaranth/hdl/__init__.py
index 130ccb6..4790fea 100644
--- a/amaranth/hdl/__init__.py
+++ b/amaranth/hdl/__init__.py
@@ -6,9 +6,9 @@ from ._ast import Format, Print, Assert, Assume, Cover
 from ._ast import IOValue, IOPort
 from ._dsl import Module
 from ._cd import DomainError, ClockDomain
-from ._ir import UnusedElaboratable, Elaboratable, DriverConflict, Fragment
+from ._ir import AlreadyElaborated, UnusedElaboratable, Elaboratable, DriverConflict, Fragment
 from ._ir import Instance, IOBufferInstance
-from ._mem import FrozenMemory, MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
+from ._mem import MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
 from ._nir import CombinationalCycle
 from ._rec import Record
 from ._xfrm import DomainRenamer, ResetInserter, EnableInserter
@@ -27,12 +27,12 @@ __all__ = [
     # _cd
     "DomainError", "ClockDomain",
     # _ir
-    "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment",
+    "AlreadyElaborated", "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment",
     "Instance", "IOBufferInstance",
     # _nir
     "CombinationalCycle",
     # _mem
-    "FrozenMemory", "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
+    "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
     # _rec
     "Record",
     # _xfrm
diff --git a/amaranth/hdl/_dsl.py b/amaranth/hdl/_dsl.py
index 5b24275..a73f20f 100644
--- a/amaranth/hdl/_dsl.py
+++ b/amaranth/hdl/_dsl.py
@@ -288,8 +288,11 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         self._domains      = {}
         self._generated    = {}
         self._src_loc      = tracer.get_src_loc()
+        self._frozen       = False
 
     def _check_context(self, construct, context):
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         if self._ctrl_context != context:
             if self._ctrl_context is None:
                 raise SyntaxError("{} is not permitted outside of {}"
@@ -616,6 +619,9 @@ class Module(_ModuleBuilderRoot, Elaboratable):
                     src_loc=src_loc))
 
     def _add_statement(self, assigns, domain, depth):
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
+
         while len(self._ctrl_stack) > self.domain._depth:
             self._pop_ctrl()
 
@@ -643,6 +649,8 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         if not hasattr(submodule, "elaborate"):
             raise TypeError("Trying to add {!r}, which does not implement .elaborate(), as "
                             "a submodule".format(submodule))
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         if name == None:
             self._anon_submodules.append((submodule, src_loc))
         else:
@@ -660,6 +668,8 @@ class Module(_ModuleBuilderRoot, Elaboratable):
     def _add_domain(self, cd):
         if cd.name in self._domains:
             raise NameError(f"Clock domain named '{cd.name}' already exists")
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         self._domains[cd.name] = cd
 
     def _flush(self):
@@ -668,6 +678,7 @@ class Module(_ModuleBuilderRoot, Elaboratable):
 
     def elaborate(self, platform):
         self._flush()
+        self._frozen = True
 
         fragment = Fragment(src_loc=self._src_loc)
         for name, (submodule, src_loc) in self._named_submodules.items():
diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py
index cbb940b..66750c1 100644
--- a/amaranth/hdl/_ir.py
+++ b/amaranth/hdl/_ir.py
@@ -3,17 +3,23 @@ from collections import defaultdict, OrderedDict
 import enum
 import warnings
 
-from .._utils import flatten, to_binary
+from .._utils import flatten, to_binary, final
 from .. import tracer, _unused
 from . import _ast, _cd, _ir, _nir
 
 
 __all__ = [
-    "UnusedElaboratable", "Elaboratable", "DuplicateElaboratable", "DriverConflict", "Fragment",
-    "Instance", "IOBufferInstance", "PortDirection", "Design", "build_netlist",
+    "AlreadyElaborated", "UnusedElaboratable", "Elaboratable", "DuplicateElaboratable",
+    "DriverConflict", "Fragment", "Instance", "IOBufferInstance", "PortDirection", "Design",
+    "build_netlist",
 ]
 
 
+@final
+class AlreadyElaborated(Exception):
+    """Exception raised when an elaboratable is being modified after elaboration."""
+
+
 class UnusedElaboratable(_unused.UnusedMustUse):
     # The warning is initially silenced. If everything that has been constructed remains unused,
     # it means the application likely crashed (with an exception, or in another way that does not
diff --git a/amaranth/hdl/_mem.py b/amaranth/hdl/_mem.py
index 4c459c1..074a370 100644
--- a/amaranth/hdl/_mem.py
+++ b/amaranth/hdl/_mem.py
@@ -4,17 +4,12 @@ from collections.abc import MutableSequence
 
 from .. import tracer
 from ._ast import *
-from ._ir import Elaboratable, Fragment
+from ._ir import Elaboratable, Fragment, AlreadyElaborated
 from ..utils import ceil_log2
 from .._utils import deprecated, final
 
 
-__all__ = ["FrozenMemory", "MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
-
-
-@final
-class FrozenMemory(Exception):
-    """Exception raised when a memory array is being modified after elaboration."""
+__all__ = ["MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
 
 
 @final
@@ -30,7 +25,7 @@ class MemoryData:
     a default value for rows that are not explicitly initialized.
 
     Changing the initial contents of a :class:`MemoryData` is only possible until it is used to
-    elaborate a memory; afterwards, attempting to do so will raise :exc:`FrozenMemory`.
+    elaborate a memory; afterwards, attempting to do so will raise :exc:`AlreadyElaborated`.
 
     .. warning::
 
@@ -101,7 +96,7 @@ class MemoryData:
 
         def __setitem__(self, index, value):
             if self._frozen:
-                raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated")
+                raise AlreadyElaborated("Cannot set 'init' on a memory that has already been elaborated")
 
             if isinstance(index, slice):
                 indices = range(*index.indices(len(self._elems)))
@@ -181,7 +176,7 @@ class MemoryData:
     @init.setter
     def init(self, init):
         if self._frozen:
-            raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated")
+            raise AlreadyElaborated("Cannot set 'init' on a memory that has already been elaborated")
         self._init = MemoryData.Init(init, shape=self._shape, depth=self._depth)
 
     def __repr__(self):
diff --git a/amaranth/lib/memory.py b/amaranth/lib/memory.py
index 5028df0..199d1e6 100644
--- a/amaranth/lib/memory.py
+++ b/amaranth/lib/memory.py
@@ -2,15 +2,14 @@ import operator
 from collections import OrderedDict
 from collections.abc import MutableSequence
 
-from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const
-from ..hdl._mem import FrozenMemory
+from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const, AlreadyElaborated
 from ..utils import ceil_log2
 from .._utils import final
 from .. import tracer
 from . import wiring, data
 
 
-__all__ = ["FrozenMemory", "Memory", "ReadPort", "WritePort"]
+__all__ = ["Memory", "ReadPort", "WritePort"]
 
 
 class Memory(wiring.Component):
@@ -24,7 +23,7 @@ class Memory(wiring.Component):
     the :ref:`elaborate <lang-elaboration>` method.
 
     Adding ports or changing initial contents of a :class:`Memory` is only possible until it is
-    elaborated; afterwards, attempting to do so will raise :class:`~amaranth.hdl.FrozenMemory`.
+    elaborated; afterwards, attempting to do so will raise :class:`~amaranth.hdl.AlreadyElaborated`.
 
     Platform overrides
     ------------------
@@ -121,7 +120,7 @@ class Memory(wiring.Component):
         :class:`ReadPort`
         """
         if self._frozen:
-            raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated")
+            raise AlreadyElaborated("Cannot add a memory port to a memory that has already been elaborated")
         signature = ReadPort.Signature(shape=self.shape, addr_width=ceil_log2(self.depth))
         return ReadPort(signature, memory=self, domain=domain, transparent_for=transparent_for,
                         src_loc_at=1 + src_loc_at)
@@ -146,7 +145,7 @@ class Memory(wiring.Component):
         :class:`WritePort`
         """
         if self._frozen:
-            raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated")
+            raise AlreadyElaborated("Cannot add a memory port to a memory that has already been elaborated")
         signature = WritePort.Signature(
             shape=self.shape, addr_width=ceil_log2(self.depth), granularity=granularity)
         return WritePort(signature, memory=self, domain=domain,
diff --git a/amaranth/vendor/_xilinx.py b/amaranth/vendor/_xilinx.py
index e762839..e5dec54 100644
--- a/amaranth/vendor/_xilinx.py
+++ b/amaranth/vendor/_xilinx.py
@@ -1214,7 +1214,7 @@ class XilinxPlatform(TemplatedPlatform):
     def get_ff_sync(self, ff_sync):
         m = Module()
         flops = [Signal(ff_sync.i.shape(), name=f"stage{index}",
-                        init=ff_sync._reset, reset_less=ff_sync._reset_less,
+                        init=ff_sync._init, reset_less=ff_sync._reset_less,
                         attrs={"ASYNC_REG": "TRUE"})
                  for index in range(ff_sync._stages)]
         if self.toolchain == "Vivado":
diff --git a/docs/stdlib/memory.rst b/docs/stdlib/memory.rst
index ad2dc42..0bb74fb 100644
--- a/docs/stdlib/memory.rst
+++ b/docs/stdlib/memory.rst
@@ -181,7 +181,8 @@ Simulation
 Memory description
 ==================
 
-.. autoexception:: amaranth.hdl.FrozenMemory
+.. autoexception:: amaranth.hdl.AlreadyElaborated
+   :noindex:
 
 .. autoclass:: amaranth.hdl.MemoryData
 


Test patch:
diff --git a/tests/test_hdl_dsl.py b/tests/test_hdl_dsl.py
index 68a4522..a255ff6 100644
--- a/tests/test_hdl_dsl.py
+++ b/tests/test_hdl_dsl.py
@@ -6,6 +6,7 @@ from collections import OrderedDict
 from amaranth.hdl._ast import *
 from amaranth.hdl._cd import *
 from amaranth.hdl._dsl import *
+from amaranth.hdl._ir import *
 from amaranth.lib.enum import Enum
 
 from .utils import *
@@ -975,3 +976,35 @@ class DSLTestCase(FHDLTestCase):
                 r"^Domain name should not be prefixed with 'cd_' in `m.domains`, "
                 r"use `m.domains.rx = ...` instead$"):
             m.domains.cd_rx = ClockDomain()
+
+    def test_freeze(self):
+        a = Signal()
+        m = Module()
+        f = Fragment.get(m, None)
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.d.comb += a.eq(1)
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.If(a):
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.Switch(a):
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.FSM():
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.submodules.a = Module()
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.domains.sync = ClockDomain()
diff --git a/tests/test_lib_memory.py b/tests/test_lib_memory.py
index b2b7bde..3700613 100644
--- a/tests/test_lib_memory.py
+++ b/tests/test_lib_memory.py
@@ -439,15 +439,15 @@ class MemoryTestCase(FHDLTestCase):
         m = memory.Memory(shape=unsigned(8), depth=4, init=[])
         m.write_port()
         m.elaborate(None)
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.write_port()
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.read_port()
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init = [1, 2, 3, 4]
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init[0] = 1