amaranth icon indicating copy to clipboard operation
amaranth copied to clipboard

Emit a warning when an Assign expression is not added to a domain

Open kbeckmann opened this issue 4 years ago • 4 comments

Assigning a signal has to be done in a domain, e.g. comb: m.d.comb += signal.eq(expression), however it is easy to forget and just write signal.eq(expression). This will lead to a silent error as no warning is produced.

kbeckmann avatar Jul 06 '20 13:07 kbeckmann

For context, I tried implementing this and it didn't work out well due to the way "fragment transformers" are currently used. We should get rid of those and then implement this.

whitequark avatar Jul 06 '20 13:07 whitequark

Is this still the case?

awygle avatar Nov 07 '20 19:11 awygle

Sadly, yes. Most of the AST transformation stuff should be ripped out and replaced with a fairly typical lexical scope mechanism. That would make elaboration several times faster, make local clock domains and module flattening actually reliable, and avoid needlessly creating Assign statements that are then deleted and create spurious warnings.

That is a lot of work though. :/ We could probably also add a workaround that ignores Assigns created by nMigen itself by looking at the path or something.

whitequark avatar Nov 07 '20 19:11 whitequark

Okay, @bracketmaster relays it from the folks at Chip11 that this issue was the biggest pain point for them while learning. I think that's a solid indication that I should just add a workaround without waiting for the massive refactor to happen. Let's fix this in 0.4.

whitequark avatar Nov 08 '20 02:11 whitequark