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

Use edition = "2024" in the compiler #129636

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Aug 27, 2024

Self-explanatory.

This involves changing all of the compiler crates' edition to 2024 (except for codegen_gcc and codegen_cranelift and pattern_analysis), and massively simplifying opaques and their lifetimes. I think the overwhelming majority of functions look significantly better now.

There are some other changes sprinkled in too, such as how env var functions are now unsafe.

@rustbot
Copy link
Collaborator

rustbot commented Aug 27, 2024

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 27, 2024
@rust-log-analyzer

This comment has been minimized.

@compiler-errors compiler-errors force-pushed the ed2024 branch 2 times, most recently from 2d3f18c to 47c874d Compare August 27, 2024 02:09
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@traviscross traviscross left a comment

Choose a reason for hiding this comment

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

This all looks right to me.

It's gratifying to see how much the Lifetime Capture Rules 2024 really clean things up, in particular by allowing lifetime elision to be used in many more places.

@@ -1294,7 +1294,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
// responsibility of lowering. This may create a mismatch between the resolution
// AST found (`region_def_id`) which points to HRTB, and what HIR allows.
// ```
// fn foo(x: impl for<'a> Trait<'a, Assoc = impl Copy + 'a>) {}
// fn foo(x: impl for<'a> Trait<'a, Assoc = impl Copy >) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// fn foo(x: impl for<'a> Trait<'a, Assoc = impl Copy >) {}
// fn foo(x: impl for<'a> Trait<'a, Assoc = impl Copy>) {}

compiler/rustc_hir_analysis/src/variance/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/upvar.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/ty.rs Outdated Show resolved Hide resolved
compiler/rustc_pattern_analysis/src/lib.rs Outdated Show resolved Hide resolved
@traviscross traviscross added the A-edition-2024 Area: The 2024 edition label Aug 27, 2024
@bors
Copy link
Contributor

bors commented Aug 28, 2024

☔ The latest upstream changes (presumably #129665) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member Author

I think this is ready.

@compiler-errors compiler-errors marked this pull request as ready for review September 18, 2024 16:30
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2024

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred in coverage instrumentation.

cc @Zalathar

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in compiler/rustc_sanitizers

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the core trait solver

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

@compiler-errors
Copy link
Member Author

r? compiler

@compiler-errors compiler-errors force-pushed the ed2024 branch 2 times, most recently from d4e9137 to b7420ee Compare September 18, 2024 16:37
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

can we remove Captures now that we have explicit use syntax?

I thought tidy requires safety comments for unsafe blocks? 🤔 I feel somewhat :/ about wrapping set_var in an unsafe block without explaining why that's correct. I guess it's always SAFETY: rustc is single threaded up until this point? I am not totally confident that this holds for all unsafe blocks, so alternatively, please open an issue tracking the fact that we may unsoundly use [set|remove]_env in the compiler

@lcnr
Copy link
Contributor

lcnr commented Sep 19, 2024

apart from that r=me, cc @rust-lang/compiler @rust-lang/compiler-contributors

@compiler-errors
Copy link
Member Author

compiler-errors commented Sep 19, 2024

can we remove Captures now that we have explicit use syntax?

I'll have to wait until use<...> is stable for that to be doable. I've hestitated to touch anything having to do with r-a for stability reasons. I can open a draft PR on top of this converting the last crate to ed 2024, and deleting Captures from the pattern analysis crate outright.

I am not totally confident that this holds for all unsafe blocks, so alternatively, please open an issue tracking the fact that we may unsoundly use [set|get]_env in the compiler

That's a good point. Yeah, I'll open a tracking issue for that (and enforcing // SAFETY in the compiler in general). and see if anyone would like to chime in on the safety. I'm not totally qualified to make these sort of libs/safety judgements, but also this is no more unsafe than it was on edition 2021, just more explicit.

@saethlin
Copy link
Member

I thought tidy requires safety comments for unsafe blocks? 🤔

We have a handful of unsafe blocks without comments, such as

let ptr = self.raw.as_mut_ptr();
unsafe { (&mut *ptr.add(ai), &mut *ptr.add(bi), &mut *ptr.add(ci)) }

pub(super) fn pointer_raw(&self) -> NonNull<P::Target> {
self.packed.map_addr(|addr| unsafe { NonZero::new_unchecked(addr.get() << T::BITS) })
}

Considering the strange way we handle formatting, I wouldn't speculate about what tidy is doing here.

@RalfJung
Copy link
Member

but also this is no more unsafe than it was on edition 2021, just more explicit.

Well the entire point of this is to give everyone a chance to audit their use of set_env. If not even rustc bothers with thta, we can hardly expect any better from our users...

@@ -51,9 +51,13 @@ fn detect_llvm_link() -> (&'static str, &'static str) {
fn restore_library_path() {
let key = tracked_env_var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
if let Some(env) = tracked_env_var_os("REAL_LIBRARY_PATH") {
env::set_var(&key, env);
unsafe {
Copy link
Member

Choose a reason for hiding this comment

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

This one (and the others that are not immediately obvious) should get a FIXME, like the warning suggests.

@@ -3,7 +3,9 @@ use super::UnstableFeatures;
#[test]
fn rustc_bootstrap_parsing() {
let is_bootstrap = |env, krate| {
std::env::set_var("RUSTC_BOOTSTRAP", env);
unsafe {
std::env::set_var("RUSTC_BOOTSTRAP", env);
Copy link
Member

Choose a reason for hiding this comment

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

This would affect other concurrent tests, right?

@bors
Copy link
Contributor

bors commented Sep 21, 2024

☔ The latest upstream changes (presumably #129283) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member Author

I've opened #130672 to track those couple sketchy usages of set_var. I think this is otherwise good to go.

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Sep 21, 2024

📌 Commit d33d913 has been approved by lcnr

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 Sep 21, 2024
@lqd
Copy link
Member

lqd commented Sep 21, 2024

do you know if the rustc-crates-on-stable rmake test failed as we expect with the pattern analysis changes? I wonder if using some of cargo’s unstable features would make it fail.

The answer is no, unfortunately.

It's also unclear to me whether we can ask cargo to only check the unstable features in the crates we're building rather than all the crates in the workspace. I've tried and couldn't do it: the presence of the unstable edition 2024 feature anywhere + cargo's -Zallow-features= failed validation even on the edition 2021 crates.

TL;DR: to my knowledge, we can't easily enforce on CI to not use cargo unstable features (including edition 2024) in the following crates that need to build on stable:

  • rustc_type_ir
  • rustc_next_trait_solver
  • rustc_pattern_analysis
  • rustc_lexer

@compiler-errors
Copy link
Member Author

Hm... I probably should hold back rustc_lexer from edition 2024 as well.

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 21, 2024
@bors
Copy link
Contributor

bors commented Sep 23, 2024

☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts.

@lqd
Copy link
Member

lqd commented Oct 21, 2024

I probably should hold back rustc_lexer from edition 2024

as well as these 2 crates I've just learned r-a also uses (via #131997)

  • rustc_abi
  • rustc_parse_format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.