Conversation
|
Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred in need_type_info.rs cc @lcnr changes to the core type system cc @lcnr Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred to the CTFE machinery Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt changes to the core type system cc @lcnr This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 HIR ty lowering was modified cc @fmease |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
winning!
Let's add a refactor tracking issue for future cleanups:
replace all the <name> @ ty::AliasTy { matches with just using args: <name>_args instead
I think similarly, the assert on AliasTy is less valuable now that we go through AliasTyKind, so maybe remove the assert, third field, and make matches nicer again
let's also look at def_id: did and do a rename to make that unnecessary in a future PR.
Ideally we don't have new_from_def_id or a alias_ty_kind_from_def_id but instead call e.g. new_projection directly, also future work
I also expect that most uses of AliasTyKind::def_id should be made unnecessary via a larger rewrite of the code
is_impl_trait should be renamed to is_opaque
there are uses of tcx.mk_ty_from_kind which should be a more specific constructor method
please rebase -> perf -> r=me if the impact is as expected from the size change test
| /// During codegen, `interner.type_of(def_id)` can be used to get the type of the | ||
| /// underlying type if the type is an opaque. |
There was a problem hiding this comment.
that comment feels wrong here
should be on Opaque
also vibes: I prefer keeping comments on the variant if there's just a single field, otherwise the formatting is kinda /
There was a problem hiding this comment.
I like it more on the variant, but changed anyway.
There was a problem hiding this comment.
I think I fixed the docs, can you double check?
compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
Outdated
Show resolved
Hide resolved
|
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 try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (5340284): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.6%, secondary 0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 488.486s -> 488.964s (0.10%) |
|
@bors rollup=never |
Add `#[track_caller]` to some functions which can panic b/c of the caller's wrong index (this helped me debug some mistakes I did while refactoring `ty::Alias`)
|
@bors r=lcnr |
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 49b6ac0 (parent) -> c3bd628 (this PR) Test differencesShow 98 test diffs98 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard c3bd6289f65fa7210c50bdeeb0ed4541caa33d2d --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 (c3bd628): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.0%, secondary 1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 1.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
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: 487.434s -> 487.084s (-0.07%) |
View all comments
This PR changes the following alias-related types from this:
Into this:
... and then does a thousand other changes to accommodate for this change everywhere.
This brings us closer to being able to have
AliasTyKinds which don't require aDefId(and thus can be more easily created, etc). Although notably we depend on bothAliasTyKind -> DefIdandDefId -> AliasTyKindconversions in a bunch of places still.r? lcnr
A lot of these changes were done either by search & replace (via
ast-grep) or on auto pilot, so I'm not quite sure I didn't mess up somewhere, but at least tests pass...