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

Don't use macros to derive ToJson impls #72

Open
RyanGlScott opened this issue Jul 18, 2024 · 0 comments
Open

Don't use macros to derive ToJson impls #72

RyanGlScott opened this issue Jul 18, 2024 · 0 comments

Comments

@RyanGlScott
Copy link
Contributor

Currently, mir-json makes use of two macros to automatically derive ToJson implementations for several types based on their variant names. Here is one place where such a macro is used:

basic_json_enum_impl!(mir::CastKind);

As a result, the CastKind FloatToFloat will be rendered as the string "FloatToFloat" in JSON, PtrToPtr will be rendered as "PtrToPtr", and so on.

This approach, while terse, has the downside that if the variant names change between different versions of the rustc API, then the JSON schema used in the ToJson impls will silently change. This actually matters in practice. For instannce, the version of rustc that mir-json currently pins against (nightly-2023-01-23) has a CastKind named Pointer, but in version 1.79.0 of rustc, Pointer is now named PointerCoercion instead, which means that downstream tools (e.g., Crucible) would need to parse the corresponding JSON differently. I wouldn't have been aware of this fact unless I bothered to look it up!

To avoid the possibility of nasty surprises like this, I propose that we avoid the use of macros like basic_json_enum_impl! and instead hand-write all of the ToJson impls, using a match expression to ensure that all possible variants are covered. This is more verbose, but it does mean that if future versions of rustc change the variant names, then we will be notified immediately when upgrading rustc versions, rather than having to play detective after the fact to figure out what changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant