Skip to content

Add an RFC for ValueCastable formatting. #58

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

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

wanda-phi
Copy link
Member

@whitequark whitequark added meta:nominated Nominated for discussion on the next relevant meeting area:core RFC affecting APIs in amaranth-lang/amaranth labels Mar 13, 2024
@whitequark
Copy link
Member

Also, I think lib.data and lib.enum are kind of weird as motivation goes, since they should not be using this facility either at all or at least this facility alone, as they need to affect the generated Verilog (by enriching it with additional wires or attributes respectively).

@wanda-phi
Copy link
Member Author

Also, I think lib.data and lib.enum are kind of weird as motivation goes, since they should not be using this facility either at all or at least this facility alone, as they need to affect the generated Verilog (by enriching it with additional wires or attributes respectively).

I feel these are orthogonal concerns. Regardless of what happens in Verilog, you still want something to happen when these are printed in simulation, perhaps even something further overridable by the user on a Struct subclass.

However, if you wish to postpone this RFC until we come up with holistic design for lib.data / lib.enum formatting and _repr replacement, I'm very much fine with this decision.

@whitequark
Copy link
Member

whitequark commented Mar 13, 2024

However, if you wish to postpone this RFC until we come up with holistic design for lib.data / lib.enum formatting and _repr replacement, I'm very much fine with this decision.

I think I'm happy to proceed for the reason you stated, I just feel like the motivation section should put more emphasis on non-magical user-defined data types like lib.fixed.

Also you make a good point re: subclasses, formatting should definitely be overridable for Struct and especially Union (where the default one doesn't make a huge amount of sense).

@whitequark
Copy link
Member

One thing I'm wondering about for lib.data is: OK, we can convert stuff to strings. But if we're going to put them into VCDs or something, that's actually a huge pain to look at for all but simplest structures, and something like Rust's :? facility that lets you concatenate snippets with predefined format to make complex representations might be a better fit.

@wanda-phi
Copy link
Member Author

However, if you wish to postpone this RFC until we come up with holistic design for lib.data / lib.enum formatting and _repr replacement, I'm very much fine with this decision.

I think I'm happy to proceed for the reason you stated, I just feel like the motivation section should put more emphasis on non-magical user-defined data types like lib.fixed.

Also you make a good point re: subclasses, formatting should definitely be overridable for Struct and especially Union (where the default one doesn't make a huge amount of sense).

The problem with lib.fixed formatting specifically is that it is not reasonably implementable in the way you'd want (eg. as a decimal number, or supporting all the weird format description stuff)

... I guess I can do some simple binary-only printout?

@wanda-phi wanda-phi force-pushed the valuecastable-format branch 3 times, most recently from 590d237 to 5f35006 Compare March 13, 2024 15:10
@wanda-phi
Copy link
Member Author

OK, I have replaced the guide example wholesale.

@whitequark
Copy link
Member

whitequark commented Mar 13, 2024

The problem with lib.fixed formatting specifically is that it is not reasonably implementable in the way you'd want (eg. as a decimal number, or supporting all the weird format description stuff)

I suppose not, but we want some formatting for fixpoint numbers that's better than just "print the number of LSBs in decimal" because that's atrocious. It would have to be implemented by the means of this RFC or something similar.

@whitequark
Copy link
Member

We have discussed this RFC on the 2024-03-25 weekly meeting. The disposition was to merge.

We have not reached a consensus for the format character to be used for the !v syntax, but it should be using Value.cast() for conversion. The syntax will be further bikeshedded in the tracking issue, defaulting to !v if no consensus emerges there still.

@whitequark whitequark removed the meta:nominated Nominated for discussion on the next relevant meeting label Mar 25, 2024
@wanda-phi wanda-phi force-pushed the valuecastable-format branch from c50555f to 737ad61 Compare March 25, 2024 18:12
@whitequark whitequark merged commit f312f68 into amaranth-lang:main Mar 25, 2024
@wanda-phi wanda-phi deleted the valuecastable-format branch March 25, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core RFC affecting APIs in amaranth-lang/amaranth
Development

Successfully merging this pull request may close these issues.

2 participants