Skip to content

Conversation

nnethercote
Copy link
Contributor

This PR implements the renaming described in rust-lang/compiler-team#699.

r? @compiler-errors

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 18, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 18, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in need_type_info.rs

cc @lcnr

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

@nnethercote
Copy link
Contributor Author

nnethercote commented Dec 18, 2023

Apologies for the massively tedious review. I split it into lots of commits so that each commit (except the last few) does exactly one change. And I already have conflicts since I started this last night, lol.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me after responding to some questions + one nit

@@ -200,18 +200,11 @@ pub(crate) fn run_fat(
modules: Vec<FatLtoInput<LlvmCodegenBackend>>,
cached_modules: Vec<(SerializedModule<ModuleBuffer>, WorkProduct)>,
) -> Result<LtoModuleCodegen<LlvmCodegenBackend>, FatalError> {
let diag_handler = cgcx.create_diag_handler();
let (symbols_below_threshold, upstream_modules) = prepare_lto(cgcx, &diag_handler)?;
let dcx = cgcx.create_dcx();
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should be create_diag_ctxt or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why function names need to be different to variable names. Consistency is good.

Copy link
Member

Choose a reason for hiding this comment

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

It's consistent with variable names, but not the type name. I consider it more normal for a constructor function to be called create_type_name, personally.

@nnethercote
Copy link
Contributor Author

Thank you for the fast review!

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Dec 18, 2023

📌 Commit 9449218 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 Dec 18, 2023
@rust-log-analyzer

This comment has been minimized.

@nnethercote
Copy link
Contributor Author

I fixed the rustfmt failure.

@bors r=compiler-errors p=1

High-priority because it's conflict prone.

@bors
Copy link
Collaborator

bors commented Dec 18, 2023

📌 Commit 1e831e3 has been approved by compiler-errors

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 18, 2023

⌛ Testing commit 1e831e3 with merge cda4736...

@bors
Copy link
Collaborator

bors commented Dec 18, 2023

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

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cda4736): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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)
1.1% [0.7%, 1.9%] 3
Regressions ❌
(secondary)
2.8% [1.5%, 4.7%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.1% [0.7%, 1.9%] 3

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.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)
1.7% [1.7%, 1.7%] 1
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-0.5%, 0.7%] 2

Binary size

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

Bootstrap: 671.683s -> 673.091s (0.21%)
Artifact size: 312.53 MiB -> 312.52 MiB (-0.00%)

adpaco-aws pushed a commit to model-checking/kani that referenced this pull request Dec 20, 2023
Fixes needed due to renaming of a few items:
 - rust-lang/rust#119063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-rustdoc Relevant to the rustdoc 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