Add basic Serde serialization capabilities to Stable MIR by sskeirik · Pull Request #126963 · rust-lang/rust

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Jun 25, 2024

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jun 27, 2024

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Jun 27, 2024
…ive, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 27, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jun 27, 2024
…ive, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 27, 2024

jhpratt added a commit to jhpratt/rust that referenced this pull request

Jun 28, 2024
…ive, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jun 28, 2024
…ive, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Jun 28, 2024
…ive, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126963 (Add basic Serde serialization capabilities to Stable MIR)
 - rust-lang#127015 (Switch back `non_local_definitions` lint to allow-by-default)
 - rust-lang#127016 (docs: check if the disambiguator matches its suffix)
 - rust-lang#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)
 - rust-lang#127041 (Migrate `run-make/override-aliased-flags` to `rmake.rs`)
 - rust-lang#127045 (Rename `super_predicates_of` and similar queries to `explicit_*` to note that they're not elaborated)
 - rust-lang#127075 (rustc_data_structures: Explicitly check for 64-bit atomics support)

r? `@ghost`
`@rustbot` modify labels: rollup

@bors bors added the S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

label

Jun 28, 2024

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jun 28, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024
…e, r=oli-obk

Add basic Serde serialization capabilities to Stable MIR

This PR adds basic Serde serialization capabilities to Stable MIR. It is intentionally minimal (just wrapping all stable MIR types with a Serde `derive`), so that any important design decisions can be discussed before going further. A simple test is included with this PR to validate that JSON can actually be emitted.

## Notes

When I wrapped the Stable MIR error types in `compiler/stable_mir/src/error.rs`, it caused test failures (though I'm not sure why) so I backed those out.

## Future Work

So, this PR will support serializing basic stable MIR, but it _does not_ support serializing interned values beneath `Ty`s and `AllocId`s, etc... My current thinking about how to handle this is as follows:

1.  Add new `visited_X` fields to the `Tables` struct for each interned category of interest.

2.  As serialization is occuring, serialize interned values as usual _and_ also record the interned value we referenced in `visited_X`.

    (Possibly) In addition, if an interned value recursively references other interned values, record those interned values as well.

3.  Teach the stable MIR `Context` how to access the `visited_X` values and expose them with wrappers in `stable_mir/src/lib.rs` to users (e.g. to serialize and/or further analyze them).

### Pros

This approach does not commit to any specific serialization format regarding interned values or other more complex cases, which avoids us locking into any behaviors that may not be desired long-term.

### Cons

The user will need to manually handle serializing interned values.

### Alternatives

1.  We can directly provide access to the underlying `Tables` maps for interned values; the disadvantage of this approach is that it either requires extra processing for users to filter out to only use the values that they need _or_ users may serialize extra values that they don't need. The advantage is that the implementation is even simpler. The other pros/cons are similar to the above.

2.  We can directly serialize interned values by expanding them in-place. The pro is that this may make some basic inputs easier to consume. However, the cons are that there will need to be special provisions for dealing with cyclical values on both the producer and consumer _and_ global values will possibly need to be de-duplicated on the consumer side.

@bors bors added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Jun 28, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 28, 2024

@bors bors added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jul 3, 2024

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jul 25, 2024