Modify MustUse to iterate the stack frame and collate options from all frames.

[robtaylor]

Options in closer frames take precedence

[codecov[bot]]

Codecov Report

:x: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 91.24%. Comparing base (5689b6b) to head (5d82323). :warning: Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
amaranth/_utils.py	90.00%	0 Missing and 1 partial :warning:

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1621   +/-   ##
=======================================
  Coverage   91.24%   91.24%           
=======================================
  Files          44       44           
  Lines       11394    11411   +17     
  Branches     2223     2226    +3     
=======================================
+ Hits        10396    10412   +16     
  Misses        836      836           
- Partials      162      163    +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[whitequark]

Oh, the other potential problem is that by capturing the callstack you will prevent any of the objects referenced by any of the frames from being collected before the entire design is garbage-collected. I think this doesn't matter for batch applications, but it will probably bite someone pretty hard eventually.

[robtaylor]

I'm undecided on this change overall. I agree that a better way to manage the noise from UnusedElaboratable is needed, but I'm not sure this is the best approach.

The main problem that gives me pause is that the semantics is so vaguely defined. Why does the outermost frame take priority, for example?

The reasoning here is that it would be the user's code closest to the error that would control the output, which seemed the most logical choice at the time. but it may indeed make more sense that its the 'top level' that controls it.

Why should we not just follow the super() call chain in the constructor instead of going through the entire callstack?

I don't want to let this PR rot but I do want at least some discussion of the exact semantics to happen first. People will come to rely on it, and changing it later will become difficult.

agreed. Its not obvious what the best policy is. That said, I think on reflection that following the call chain of the constructor could be confusing as the 'point' where a user gets the list of warnings may be quite separate in space and time from any given amaranth object.

[robtaylor]

oh, good catch. Does it work to do the work of checking options at MustUse creation instead of at del?

On Mon, 4 Aug 2025 at 23:45, Catherine @.***> wrote:

whitequark left a comment (amaranth-lang/amaranth#1621) https://github.com/amaranth-lang/amaranth/pull/1621#issuecomment-3152649396

Oh, the other potential problem is that by capturing the callstack you will prevent any of the objects referenced by any of the frames from being collected before the entire design is garbage-collected. I think this doesn't matter for batch applications, but it will probably bite someone pretty hard eventually.

— Reply to this email directly, view it on GitHub https://github.com/amaranth-lang/amaranth/pull/1621#issuecomment-3152649396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4O4H4RWWZ75U4MEMQ7TD3L7PBJAVCNFSM6AAAAACCXVJZBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNJSGY2DSMZZGY . You are receiving this because you authored the thread.Message ID: @.***>

[whitequark]

At the time I implemented the existing code, I was concerned about the overhead of using linecache, since it takes a noticeable amount of time to grab the source code. This is exponentially worse with your PR since it will do this for every frame in the call stack, which is potentially a very large number. The lines do get cached at least.

A compromise could be to turn a traceback into a list of co_filename values, which are then turned into options later. I think this does the minimal amount of work possible.

[whitequark]

By the way, regarding the "first/last wins" discussion: one caveat is that the same file can appear in the traceback multiple times, potentially in between two appearances of some other file; this is something that will be particularly acute if you use decorators.

[robtaylor]

At the time I implemented the existing code, I was concerned about the overhead of using linecache, since it takes a noticeable amount of time to grab the source code. This is exponentially worse with your PR since it will do this for every frame in the call stack, which is potentially a very large number. The lines do get cached at least.

A compromise could be to turn a traceback into a list of co_filename values, which are then turned into options later. I think this does the minimal amount of work possible.

I was thinking this too, so I measured it - surprisingly it makes little difference over the amaranth test suite, i guess as most of the sources are in local cache.

Comparing here main (first set of data) to 5d82323

Run	user (s)	system (s)	cpu %	total
1	28.58	3.65	127%	25.288
2	29.06	3.84	128%	25.589
3	28.89	3.73	127%	25.512
4	29.24	3.86	127%	25.997
4	29.4	3.77	127%	25.993
Average	29.034	3.77	127%	25.6758
Std Dev	0.318	0.085	0.004	0.312
1	28.28	3.77	127%	25.137
2	28.64	3.92	127%	25.511
3	28.56	3.95	127%	25.446
4	28.84	3.74	127%	25.465
5	29.11	3.72	127%	25.822
Average	28.686	3.82	127%	25.4762
Std Dev	0.311	0.107	0.000	0.243

[robtaylor]

@whitequark what's next steps here?

[robtaylor]

For my reference: Relevant open issues are:

• #1382
• #924

Related closed issues:

• #848
• #436

[robtaylor]

Another thing that occurred to me - We currently only show these on an exception, though arguably this is when they are least useful. It seems to me that it would be more useful to default to displaying after successful elaboration.

[whitequark]

@whitequark what's next steps here?

Apologies for the delay, let's just merge it. I'm a little uncertain if this is the right direction to go in, but if it turns out to cause issues we can just modify the code again.

[whitequark]

Happy to merge if you squash the commits in the PR!