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

Simplifications in match lowering #126835

Merged
merged 8 commits into from
Jun 29, 2024
Merged

Conversation

Nadrieril
Copy link
Member

A series of small simplifications and deduplications in the MIR lowering of patterns.

r? @matthewjasper

@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 22, 2024
Comment on lines -742 to +760
let mut next = Some(&candidate);
while let Some(candidate_ref) = next.take() {
for binding in &candidate_ref.extra_data.bindings {
// `try_to_place` may fail if it is unable to resolve the given `PlaceBuilder` inside a
// closure. In this case, we don't want to include a scrutinee place.
// `scrutinee_place_builder` will fail for destructured assignments. This is because a
// closure only captures the precise places that it will read and as a result a closure
// may not capture the entire tuple/struct and rather have individual places that will
// be read in the final MIR.
// Example:
// ```
// let foo = (0, 1);
// let c = || {
// let (v1, v2) = foo;
// };
// ```
if let Some(place) = initializer.try_to_place(self) {
visit_bindings(&[&mut candidate], |binding: &Binding<'_>| {
let local = self.var_local_id(binding.var_id, OutsideGuard);
// `try_to_place` may fail if it is unable to resolve the given
// `PlaceBuilder` inside a closure. In this case, we don't want to include
// a scrutinee place. `scrutinee_place_builder` will fail for destructured
// assignments. This is because a closure only captures the precise places
// that it will read and as a result a closure may not capture the entire
// tuple/struct and rather have individual places that will be read in the
// final MIR.
// Example:
// ```
// let foo = (0, 1);
// let c = || {
// let (v1, v2) = foo;
// };
// ```
if let Some(place) = initializer.try_to_place(self) {
let LocalInfo::User(BindingForm::Var(VarBindingForm {
opt_match_place: Some((ref mut match_place, _)),
..
})) = **self.local_decls[local].local_info.as_mut().assert_crate_local()
else {
bug!("Let binding to non-user variable.")
};
if let LocalInfo::User(BindingForm::Var(VarBindingForm {
opt_match_place: Some((ref mut match_place, _)),
..
})) = **self.local_decls[local].local_info.as_mut().assert_crate_local()
{
*match_place = Some(place);
}
}
// All of the subcandidates should bind the same locals, so we
// only visit the first one.
next = candidate_ref.subcandidates.get(0)
} else {
bug!("Let binding to non-user variable.")
};
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this isn't so much a simplification by itself; it's just making my life simpler as I try to clarify this whole thing.

@rust-log-analyzer

This comment has been minimized.

@Nadrieril Nadrieril force-pushed the reify-decision-tree branch from b408228 to beb1d35 Compare June 22, 2024 17:06
Comment on lines +216 to +217
false,
true,
Copy link
Contributor

@Zalathar Zalathar Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of thing makes me want to start adding dedicated enums to replace some of these magic booleans.

For example, I want to replace declare_let_bindings: bool with a three-way enum:

enum DeclareLetBindings {
    Yes,
    No,
    LetNotPermitted,
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened a draft PR to replace some booleans with enums: #126981.

@matthewjasper
Copy link
Contributor

I think it makes sense to merge this separately from #126981.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 27, 2024

📌 Commit beb1d35 has been approved by matthewjasper

It is now in the queue for this repository.

@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
…atthewjasper

Simplifications in match lowering

A series of small simplifications and deduplications in the MIR lowering of patterns.

r? `@matthewjasper`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2024
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - rust-lang#126963 (Add basic Serde serialization capabilities to Stable MIR)
 - rust-lang#127010 (Update browser-ui-test version to `0.18.0`)
 - 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`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 27, 2024
…atthewjasper

Simplifications in match lowering

A series of small simplifications and deduplications in the MIR lowering of patterns.

r? ``@matthewjasper``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - rust-lang#126963 (Add basic Serde serialization capabilities to Stable MIR)
 - rust-lang#127010 (Update browser-ui-test version to `0.18.0`)
 - rust-lang#127015 (Switch back `non_local_definitions` lint to allow-by-default)
 - rust-lang#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 28, 2024
…atthewjasper

Simplifications in match lowering

A series of small simplifications and deduplications in the MIR lowering of patterns.

r? ```@matthewjasper```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - 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#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - 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#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - 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#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123237 (Various rustc_codegen_ssa cleanups)
 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126835 (Simplifications in match lowering)
 - 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#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126822 (Bootstrap command refactoring: port more `Command` usages to `BootstrapCmd` (step 2))
 - rust-lang#126835 (Simplifications in match lowering)
 - rust-lang#126953 (std: separate TLS key creation from TLS access)
 - 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)
 - rust-lang#127101 (remove redundant match statement from dataflow const prop)
 - rust-lang#127102 (Rename fuchsia builder and bump Fuchsia)
 - rust-lang#127103 (Move binder and polarity parsing into `parse_generic_ty_bound`)
 - rust-lang#127108 (unify `dylib` and `bin_helpers` and create `shared_helpers::parse_value_from_args`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 806c5c1 into rust-lang:master Jun 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
Rollup merge of rust-lang#126835 - Nadrieril:reify-decision-tree, r=matthewjasper

Simplifications in match lowering

A series of small simplifications and deduplications in the MIR lowering of patterns.

r? ````@matthewjasper````
@Nadrieril Nadrieril deleted the reify-decision-tree branch June 29, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants