Extend -Zdump-mir output with two-phase borrow info by pnkfelix · Pull Request #105815 · rust-lang/rust (original) (raw)
This should cause no breakage since its just injecting a comment /*2pb*/ (and the -Zdump-mir output is unstable anyway).
…::Unique while we're at it.
r? @lcnr
(rustbot has picked a reviewer for you, use r? to override)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
generally in favor even though the representation isn't great. I am not sure how long it would take me to figure out what /* 2pb */ means. Can't quickly think of a better alternative though '^^
r? @oli-obk
@oli-obk can you spell out how much more info you think you'd want? Right now this really is just a boolean flag, so the presence/absence of a tag like /*2pb*/ is all I could think of to include.
(Maybe you're saying you'd want to know about when the reservation begins versus when the exclusive access itself actually begins, which I would agree would be superior, but I do not think that information is directly reflected in the MIR structure, and I think I'd bias towards making the dump_mir output merely reflect existing structure rather than have it attempt further deductions in the dump_mir code itself...)
(I have been musing whether there would be value in having the MIR itself use general structure where every mutable borrow is modelled as a two-phase borrow, with distinct "reserve-mut" and "exercise-the-reservation" operations. But that is a huge change that would need far broader discussion.)
Right now this really is just a boolean flag, so the presence/absence of a tag like
/*2pb*/is all I could think of to include.
Oh, I don't mean more information, just clarifying what 2pb means in a comment, similar to how we have comments for constants giving their type and potentially verbose value
r=me with either the new comments added or not ^^
@rustbot author
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
@pnkfelix Ping from triage: Can you post your status on this PR? This has sat idle for a few months.
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.
@rustbot label: +S-inactive
Status: Inactive and waiting on the author. This is often applied to closed PRs.
label
2 tasks
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…s, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool
I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=.
-Zno-jump-tables stabilization report
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows.
The tracking is rust-lang#116592.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
The behavior of this flag is well defined, and mimics the existing -fno-jump-tables option currently available with LLVM and GCC with some caveats:
- Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option.
- The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core.
- This option only enforces the crate being compiled is free of jump tables.
- No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option.
What should the flag name be?
- As introduced, this option was named
-Zno-jump-tables. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to-Cjump-tables=<bool>makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table.
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those.
No. This option is used exclusively to gate a very specific class of optimization.
Summarize the major parts of the implementation and provide links into the code (or to PRs)
- The original PR rust-lang#105812 by
@ojeda - The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026
- This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215
- Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223
Has a call-for-testing period been conducted? If so, what feedback was received?
No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue.
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
There are no outstanding issues.
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
@ojedaimplemented this feature in rust-lang#105815 as-Zno-jump-tables.@tgross35created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
There are none.
What static checks are done that are needed to prevent undefined behavior?
This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases.
In what way does this feature interact with the reference/specification, and are those edits prepared?
This adds a new cli option to rustc. The documentation is updated, and the unstable documentation cleaned up in this PR.
Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?
No.
What other unstable features may be exposed by this feature?
None.
What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.?
No support is required from other rust tooling.
Open Items
- Are there objections renaming
-Zno-jump-tablesto-Cjump-tables=<bool>? The consensus is no. - Is it desirable to keep
-Zno-jump-tablesfor a period of time? The consensus is no.
Closes rust-lang#116592
Zalathar added a commit to Zalathar/rust that referenced this pull request
…s, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool
I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=.
-Zno-jump-tables stabilization report
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows.
The tracking is rust-lang#116592.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
The behavior of this flag is well defined, and mimics the existing -fno-jump-tables option currently available with LLVM and GCC with some caveats:
- Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option.
- The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core.
- This option only enforces the crate being compiled is free of jump tables.
- No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option.
What should the flag name be?
- As introduced, this option was named
-Zno-jump-tables. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to-Cjump-tables=<bool>makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table.
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those.
No. This option is used exclusively to gate a very specific class of optimization.
Summarize the major parts of the implementation and provide links into the code (or to PRs)
- The original PR rust-lang#105812 by
@ojeda - The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026
- This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215
- Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223
Has a call-for-testing period been conducted? If so, what feedback was received?
No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue.
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
There are no outstanding issues.
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
@ojedaimplemented this feature in rust-lang#105815 as-Zno-jump-tables.@tgross35created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
There are none.
What static checks are done that are needed to prevent undefined behavior?
This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases.
In what way does this feature interact with the reference/specification, and are those edits prepared?
This adds a new cli option to rustc. The documentation is updated, and the unstable documentation cleaned up in this PR.
Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?
No.
What other unstable features may be exposed by this feature?
None.
What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.?
No support is required from other rust tooling.
Open Items
- Are there objections renaming
-Zno-jump-tablesto-Cjump-tables=<bool>? The consensus is no. - Is it desirable to keep
-Zno-jump-tablesfor a period of time? The consensus is no.
Closes rust-lang#116592
Zalathar added a commit to Zalathar/rust that referenced this pull request
…s, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool
I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=.
-Zno-jump-tables stabilization report
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows.
The tracking is rust-lang#116592.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
The behavior of this flag is well defined, and mimics the existing -fno-jump-tables option currently available with LLVM and GCC with some caveats:
- Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option.
- The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core.
- This option only enforces the crate being compiled is free of jump tables.
- No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option.
What should the flag name be?
- As introduced, this option was named
-Zno-jump-tables. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to-Cjump-tables=<bool>makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table.
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those.
No. This option is used exclusively to gate a very specific class of optimization.
Summarize the major parts of the implementation and provide links into the code (or to PRs)
- The original PR rust-lang#105812 by
@ojeda - The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026
- This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215
- Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223
Has a call-for-testing period been conducted? If so, what feedback was received?
No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue.
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
There are no outstanding issues.
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
@ojedaimplemented this feature in rust-lang#105815 as-Zno-jump-tables.@tgross35created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
There are none.
What static checks are done that are needed to prevent undefined behavior?
This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases.
In what way does this feature interact with the reference/specification, and are those edits prepared?
This adds a new cli option to rustc. The documentation is updated, and the unstable documentation cleaned up in this PR.
Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?
No.
What other unstable features may be exposed by this feature?
None.
What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.?
No support is required from other rust tooling.
Open Items
- Are there objections renaming
-Zno-jump-tablesto-Cjump-tables=<bool>? The consensus is no. - Is it desirable to keep
-Zno-jump-tablesfor a period of time? The consensus is no.
Closes rust-lang#116592
Zalathar added a commit to Zalathar/rust that referenced this pull request
…s, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool
I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=.
-Zno-jump-tables stabilization report
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows.
The tracking is rust-lang#116592.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
The behavior of this flag is well defined, and mimics the existing -fno-jump-tables option currently available with LLVM and GCC with some caveats:
- Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option.
- The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core.
- This option only enforces the crate being compiled is free of jump tables.
- No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option.
What should the flag name be?
- As introduced, this option was named
-Zno-jump-tables. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to-Cjump-tables=<bool>makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table.
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those.
No. This option is used exclusively to gate a very specific class of optimization.
Summarize the major parts of the implementation and provide links into the code (or to PRs)
- The original PR rust-lang#105812 by
@ojeda - The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026
- This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215
- Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223
Has a call-for-testing period been conducted? If so, what feedback was received?
No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue.
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
There are no outstanding issues.
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
@ojedaimplemented this feature in rust-lang#105815 as-Zno-jump-tables.@tgross35created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
There are none.
What static checks are done that are needed to prevent undefined behavior?
This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases.
In what way does this feature interact with the reference/specification, and are those edits prepared?
This adds a new cli option to rustc. The documentation is updated, and the unstable documentation cleaned up in this PR.
Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?
No.
What other unstable features may be exposed by this feature?
None.
What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.?
No support is required from other rust tooling.
Open Items
- Are there objections renaming
-Zno-jump-tablesto-Cjump-tables=<bool>? The consensus is no. - Is it desirable to keep
-Zno-jump-tablesfor a period of time? The consensus is no.
Closes rust-lang#116592
rust-timer added a commit that referenced this pull request
Rollup merge of #145974 - pmur:murp/stabilize-zno-jump-tables, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool
I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=.
-Zno-jump-tables stabilization report
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
No RFC was created for this option. This was a narrowly scoped option introduced in #105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows.
The tracking is #116592.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
The behavior of this flag is well defined, and mimics the existing -fno-jump-tables option currently available with LLVM and GCC with some caveats:
- Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option.
- The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core.
- This option only enforces the crate being compiled is free of jump tables.
- No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option.
What should the flag name be?
- As introduced, this option was named
-Zno-jump-tables. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to-Cjump-tables=<bool>makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table.
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those.
No. This option is used exclusively to gate a very specific class of optimization.
Summarize the major parts of the implementation and provide links into the code (or to PRs)
- The original PR #105812 by
@ojeda - The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026
- This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215
- Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223
Has a call-for-testing period been conducted? If so, what feedback was received?
No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue.
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
There are no outstanding issues.
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
@ojedaimplemented this feature in #105815 as-Zno-jump-tables.@tgross35created and maintained the tracking issue #116592, and provided feedback about the naming of the cli option.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
There are none.
What static checks are done that are needed to prevent undefined behavior?
This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases.
In what way does this feature interact with the reference/specification, and are those edits prepared?
This adds a new cli option to rustc. The documentation is updated, and the unstable documentation cleaned up in this PR.
Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?
No.
What other unstable features may be exposed by this feature?
None.
What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.?
No support is required from other rust tooling.
Open Items
- Are there objections renaming
-Zno-jump-tablesto-Cjump-tables=<bool>? The consensus is no. - Is it desirable to keep
-Zno-jump-tablesfor a period of time? The consensus is no.
Closes #116592