Skip to content

Conversation

whitequark
Copy link
Member

@whitequark whitequark commented Jul 23, 2023

Using sys.excepthook to silence the must-use warning has some false negatives: applications may catch the exception and then quit normally, e.g. becaue the error is well known and does not require a traceback to be shown (which would be noisy). The current implementation prints even more noise in that case.

In addition to the existing heuristic, silence the warning if nothing has been elaborated, which is almost always a reliable sign. It doesn't work if multiple designs are independently created in the application and some of them are dropped without being used, but this is unavoidable as it is not distinguishable from the mistake this warning is attempting to prevent.

Fixes #848.

@whitequark whitequark added this to the 0.4 milestone Jul 23, 2023
@codecov
Copy link

codecov bot commented Jul 23, 2023

Codecov Report

Merging #849 (176af59) into main (7a9dbc8) will decrease coverage by 0.02%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #849      +/-   ##
==========================================
- Coverage   82.80%   82.79%   -0.02%     
==========================================
  Files          52       52              
  Lines        7085     7089       +4     
  Branches     1698     1699       +1     
==========================================
+ Hits         5867     5869       +2     
  Misses       1027     1027              
- Partials      191      193       +2     
Impacted Files Coverage Δ
amaranth/_unused.py 78.57% <0.00%> (-6.05%) ⬇️
amaranth/hdl/ir.py 95.50% <100.00%> (+0.01%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Using `sys.excepthook` to silence the must-use warning has some false
negatives: applications may catch the exception and then quit
normally, e.g. becaue the error is well known and does not require
a traceback to be shown (which would be noisy). The current
implementation prints even more noise in that case.

In addition to the existing heuristic, silence the warning if
*nothing* has been elaborated, which is almost always a reliable
sign. It doesn't work if multiple designs are independently created
in the application and some of them are dropped without being used,
but this is unavoidable as it is not distinguishable from the mistake
this warning is attempting to prevent.

Fixes amaranth-lang#848.
@whitequark whitequark force-pushed the heuristic-for-mustuse branch from 4adfe76 to 176af59 Compare July 23, 2023 03:46
@whitequark whitequark changed the title hdl.ir: use heuristic for silencing warning instead of sys.excepthook hdl.ir: use additional heuristic for silencing warning Jul 23, 2023
@whitequark whitequark added this pull request to the merge queue Jul 23, 2023
Merged via the queue into amaranth-lang:main with commit a6d67f7 Jul 23, 2023
@whitequark whitequark deleted the heuristic-for-mustuse branch July 23, 2023 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Don't show UnusedElaboratable warning if not a single elaboratable was used

1 participant