Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amaranth Linter #360

Closed
awygle opened this issue Apr 20, 2020 · 4 comments
Closed

Amaranth Linter #360

awygle opened this issue Apr 20, 2020 · 4 comments

Comments

@awygle
Copy link
Contributor

awygle commented Apr 20, 2020

It would be great to have a linter which reports on things like "you are assigning signals of different widths", "you assign this comb signal in one branch of this switch but not another", or "your FSM has multiple exit transitions with the same condition".

@whitequark
Copy link
Member

My current plan for addressing this need is providing some example linters, and if the idea turns out to be popular, we can figure out some nice way to integrate them with core nMigen.

@Maykeye
Copy link

Maykeye commented Jul 30, 2022

Regarding width, fortunately Signal is not final (@final is commented out ). So for now at least it's possible to workaround by wrapping signals in python code directly like

class SafeSignal(Signal): 
 def __eq__(self, other):
    self.__check_shapes(other)
    return super().__eq__(other)

 def __check_shapes(self, other : Value):
    if hasattr(other, "shape"):
        if other.shape().width != self.shape().width:
            raise ValueError(f"{self}@{self.src_loc} and {other}@{other.src_loc} have different shapes: {self.shape()} vs {other.shape()}")

...
a : Signal = SafeSignal(8)

It will not catch all errors (Signal(8)==1024) and requires overriding lots of methods to be effective, but at least it can save from some headache

@whitequark
Copy link
Member

Regarding width, fortunately Signal is not final (@final is commented out ).

This will stop being the case once amaranth.compat is deprecated for good. We'll need to come up with a different design by that point, probably.

@whitequark
Copy link
Member

Adding a linter would need an RFC. In general, I favor good language design and built-in (always-on) diagnostics over linting, but it seems inevitable that someone will want a linter, too. This isn't something that's currently on the roadmap, so I'll go ahead and close this issue.

If you strongly feel that there should be a linter, please comment on this issue and/or write an RFC.

@whitequark whitequark closed this as not planned Won't fix, can't repro, duplicate, stale Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants