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

Move TyCtxt::mk_x to Ty::new_x where applicable #113377

Merged
merged 3 commits into from
Jul 6, 2023

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Jul 5, 2023

Part of rust-lang/compiler-team#616

turns out there's a lot of places we construct Ty this is a ridiculously huge PR :S

r? @oli-obk

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Jul 5, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

@@ -140,7 +141,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let size = u64::try_from(os_str.len()).unwrap().checked_add(1).unwrap(); // Make space for `0` terminator.
let this = self.eval_context_mut();

let arg_type = this.tcx.mk_array(this.tcx.types.u8, size);
let arg_type = Ty::new_array(this.tcx.tcx,this.tcx.types.u8, size);
Copy link
Member

Choose a reason for hiding this comment

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

subtree formatting :(

compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved
@compiler-errors
Copy link
Member

@bors r+ p=10 (very bitrotty!!)

cc subtree owners (@rust-lang/miri, @rust-lang/clippy, and @antoyo for codegen_gcc), you all will need to run rustfmt on your own after this lands.

@bors
Copy link
Contributor

bors commented Jul 5, 2023

📌 Commit 6ce1c89 has been approved by compiler-errors

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 Jul 5, 2023
@compiler-errors
Copy link
Member

(oh and @bjorn3 too, you're a subtree owner too!)

@BoxyUwU BoxyUwU assigned compiler-errors and unassigned oli-obk Jul 5, 2023
@compiler-errors
Copy link
Member

Noting as a retrospective, but not a blocker of this PR, that we have a lot of code even within the compiler/ directory which is unformatted and remains a bit ugly due to this PR (e.g. Ty::mk_unit(tcx,)).

This is not this PR's fault, but the fault of not having formatting for let-chains in-tree. I've chatted separately with @calebcartwright to understand how we can get provisional formatting into rust sooner than later now that we (T-style) have decided on a new nightly-features formatting policy 😉

@compiler-errors
Copy link
Member

Additional nice-to-haves after this PR lands (just the ones that come to mind):

  • Replace usages of Ty::new_unit with tcx.types.unit (or vice versa) and similar with the other built-in scalar types for Consistency(TM).
  • Replace usages of ty::new_ref that have a fixed mutability with new_imm_ref or new_mut_ref to shorten it and avoid having to go through ty::TyAndMutability -- new_ref should imo only be used when the mutability is being passed in or is variable or something.
  • Ideally we'd fix some of the annoying formatting blips (like a lack of spacing after tcx, in the args) before getting full let-chains support...

@bors
Copy link
Contributor

bors commented Jul 5, 2023

⌛ Testing commit 6ce1c89 with merge b3485f33f946f7556722b47529e79f82632fb229...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 6, 2023

💔 Test failed - checks-actions

@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 Jul 6, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jul 6, 2023

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Jul 6, 2023

📌 Commit deda49e has been approved by compiler-errors

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 Jul 6, 2023
@bors
Copy link
Contributor

bors commented Jul 6, 2023

⌛ Testing commit deda49e with merge 4dd1719...

@bors
Copy link
Contributor

bors commented Jul 6, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 4dd1719 to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4dd1719): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.7% [-1.1%, -0.4%] 10
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.7% [-1.1%, -0.4%] 10

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.1% [-4.3%, -1.4%] 8
Improvements ✅
(secondary)
-1.5% [-1.5%, -1.5%] 1
All ❌✅ (primary) -3.1% [-4.3%, -1.4%] 8

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 659.042s -> 656.237s (-0.43%)

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2023
…ler-errors

Move `TyCtxt::mk_x` to `Ty::new_x` where applicable

Part of rust-lang/compiler-team#616

turns out there's a lot of places we construct `Ty` this is a ridiculously huge PR :S

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants