-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
THIR patterns: Always use type str for string-constant-value nodes
#151155
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
Conversation
|
Some changes occurred in match checking cc @Nadrieril Some changes occurred in match lowering cc @Nadrieril Some changes occurred in exhaustiveness checking cc @Nadrieril |
|
This does result in a few more deref-ref round trips in built MIR, so let's see if that has measurable perf consequences: @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
THIR patterns: Always use type `str` for string-constant-value nodes
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
THIR patterns: Always use type `str` for string-constant-value nodes
|
Oh, very nice! Loving your small improvements :) r? me r=me if perf is good |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f0bde62): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (primary -0.1%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 473.873s -> 472.953s (-0.19%) |
|
@bors r=Nadrieril rollup=maybe |
THIR patterns: Always use type `str` for string-constant-value nodes Historically, constants and literals of type `&str` have been represented in THIR patterns as `PatKind::Const` nodes with type `&str`. That's fine for stable Rust, but `feature(deref_patterns)` also created a need to have string literal patterns of type `str` in some cases, which resulted in a number of additional special cases and inconsistencies in typechecking and in HIR-to-THIR-to-MIR lowering of patterns. We can avoid several of those special cases by having THIR treat string-constant-values as fundamentally being of type `str`, and then using `PatKind::Deref` to represent the additional `&` layer in the common case where it is needed. This allows bare `str` patterns to require very little special treatment. Existing tests should already do a good job of demonstrating that this implementation change does not affect the stable language.
THIR patterns: Always use type `str` for string-constant-value nodes Historically, constants and literals of type `&str` have been represented in THIR patterns as `PatKind::Const` nodes with type `&str`. That's fine for stable Rust, but `feature(deref_patterns)` also created a need to have string literal patterns of type `str` in some cases, which resulted in a number of additional special cases and inconsistencies in typechecking and in HIR-to-THIR-to-MIR lowering of patterns. We can avoid several of those special cases by having THIR treat string-constant-values as fundamentally being of type `str`, and then using `PatKind::Deref` to represent the additional `&` layer in the common case where it is needed. This allows bare `str` patterns to require very little special treatment. Existing tests should already do a good job of demonstrating that this implementation change does not affect the stable language.
|
Commit 0777344 has been unapproved. |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r=Nadrieril |
THIR patterns: Always use type `str` for string-constant-value nodes Historically, constants and literals of type `&str` have been represented in THIR patterns as `PatKind::Const` nodes with type `&str`. That's fine for stable Rust, but `feature(deref_patterns)` also created a need to have string literal patterns of type `str` in some cases, which resulted in a number of additional special cases and inconsistencies in typechecking and in HIR-to-THIR-to-MIR lowering of patterns. We can avoid several of those special cases by having THIR treat string-constant-values as fundamentally being of type `str`, and then using `PatKind::Deref` to represent the additional `&` layer in the common case where it is needed. This allows bare `str` patterns to require very little special treatment. Existing tests should already do a good job of demonstrating that this implementation change does not affect the stable language.
THIR patterns: Always use type `str` for string-constant-value nodes Historically, constants and literals of type `&str` have been represented in THIR patterns as `PatKind::Const` nodes with type `&str`. That's fine for stable Rust, but `feature(deref_patterns)` also created a need to have string literal patterns of type `str` in some cases, which resulted in a number of additional special cases and inconsistencies in typechecking and in HIR-to-THIR-to-MIR lowering of patterns. We can avoid several of those special cases by having THIR treat string-constant-values as fundamentally being of type `str`, and then using `PatKind::Deref` to represent the additional `&` layer in the common case where it is needed. This allows bare `str` patterns to require very little special treatment. Existing tests should already do a good job of demonstrating that this implementation change does not affect the stable language.
Rollup of 4 pull requests Successful merges: - #151155 (THIR patterns: Always use type `str` for string-constant-value nodes) - #151166 (fix: Do not delay E0107 when there exists an assoc ty with the same name) - #151172 (Use default field values in a few more cases) - #151185 (Disable `dump-ice-to-disk` on `i686-pc-windows-msvc`) r? @ghost
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing bcf787a (parent) -> d2015e2 (this PR) Test differencesShow 58 test diffsStage 1
Stage 2
Additionally, 56 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard d2015e2359d5d0b154c2b192d4039f9b5711fcdc --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (d2015e2): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 3.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 473.462s -> 471.749s (-0.36%) |
Historically, constants and literals of type
&strhave been represented in THIR patterns asPatKind::Constnodes with type&str.That's fine for stable Rust, but
feature(deref_patterns)also created a need to have string literal patterns of typestrin some cases, which resulted in a number of additional special cases and inconsistencies in typechecking and in HIR-to-THIR-to-MIR lowering of patterns.We can avoid several of those special cases by having THIR treat string-constant-values as fundamentally being of type
str, and then usingPatKind::Derefto represent the additional&layer in the common case where it is needed. This allows barestrpatterns to require very little special treatment.Existing tests should already do a good job of demonstrating that this implementation change does not affect the stable language.