Repository: amaranth-lang/amaranth (python)

Issue:
Missing return in elaborate results in cryptic error
A missing return gives a useful warning: 

`UserWarning: .elaborate() returned None; missing return statement?`

but this is then followed by a cryptic error:
 
`TypeError: Object None is not an 'Elaboratable' nor 'Fragment'`

Would it be better to raise an error instead of a warning?



Pull request description:
Improve error raised when returning None from `elaborate`
Fixes #1569.

Patch:
diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py
index 0bdbb97..1370bde 100644
--- a/amaranth/hdl/_ir.py
+++ b/amaranth/hdl/_ir.py
@@ -49,8 +49,8 @@ class DomainRequirementFailed(Exception):
 class Fragment:
     @staticmethod
     def get(obj, platform):
-        code = None
         origins = []
+        returned_by = ""
         while True:
             if isinstance(obj, Fragment):
                 if hasattr(obj, "origins"):
@@ -58,19 +58,15 @@ class Fragment:
                 return obj
             elif isinstance(obj, Elaboratable):
                 code = obj.elaborate.__code__
+                returned_by = f", returned by {code.co_filename}:{code.co_firstlineno}"
                 UnusedElaboratable._MustUse__silence = False
                 obj._MustUse__used = True
                 new_obj = obj.elaborate(platform)
             else:
-                raise TypeError(f"Object {obj!r} is not an 'Elaboratable' nor 'Fragment'")
+                raise TypeError(
+                    f"Object {obj!r} is not an 'Elaboratable' nor 'Fragment'{returned_by}")
             if new_obj is obj:
-                raise RecursionError(f"Object {obj!r} elaborates to itself")
-            if new_obj is None and code is not None:
-                warnings.warn_explicit(
-                    message=".elaborate() returned None; missing return statement?",
-                    category=UserWarning,
-                    filename=code.co_filename,
-                    lineno=code.co_firstlineno)
+                raise RecursionError(f"Object {obj!r} elaborates to itself{returned_by}")
             origins.append(obj)
             obj = new_obj
 


Test patch:
diff --git a/tests/test_hdl_ir.py b/tests/test_hdl_ir.py
index bbfc86c..c017f38 100644
--- a/tests/test_hdl_ir.py
+++ b/tests/test_hdl_ir.py
@@ -31,15 +31,13 @@ class FragmentGetTestCase(FHDLTestCase):
                 r"^Object None is not an 'Elaboratable' nor 'Fragment'$"):
             Fragment.get(None, platform=None)
 
-        with self.assertWarnsRegex(UserWarning,
-                r"^\.elaborate\(\) returned None; missing return statement\?$"):
-            with self.assertRaisesRegex(TypeError,
-                    r"^Object None is not an 'Elaboratable' nor 'Fragment'$"):
-                Fragment.get(ElaboratesToNone(), platform=None)
+        with self.assertRaisesRegex(TypeError,
+                r"^Object None is not an 'Elaboratable' nor 'Fragment', returned by .+?:19$"):
+            Fragment.get(ElaboratesToNone(), platform=None)
 
     def test_get_wrong_self(self):
         with self.assertRaisesRegex(RecursionError,
-                r"^Object <.+?ElaboratesToSelf.+?> elaborates to itself$"):
+                r"^Object <.+?ElaboratesToSelf.+?> elaborates to itself, returned by .+?:24$"):
             Fragment.get(ElaboratesToSelf(), platform=None)