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

Add TODO comment to unsafe env modification #126019

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Jun 5, 2024

Addresses #124636 (comment).

I think that the diff display regresses a little, because it's no longer showing the + to show where the unsafe {} is added. I think it's still fine.

Tracking:

r? @RalfJung

@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. labels Jun 5, 2024
@RalfJung
Copy link
Member

RalfJung commented Jun 5, 2024

Sorry, I can't take this review currently.
r? compiler

@rustbot rustbot assigned fee1-dead and unassigned RalfJung Jun 5, 2024
@bors
Copy link
Contributor

bors commented Jun 6, 2024

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

@tbu- tbu- force-pushed the pr_unsafe_env_fixme branch 2 times, most recently from 8f82878 to 12a98c2 Compare June 11, 2024 15:47
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 11, 2024
@tbu-
Copy link
Contributor Author

tbu- commented Jun 11, 2024

@fee1-dead Changed it to TODO, fixed tidy. Should be ready for review now.

@tbu- tbu- changed the title Add FIXME comment to unsafe env modification Add TODO comment to unsafe env modification Jun 11, 2024
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

LGTM, one last nit.

src/tools/tidy/src/style.rs Show resolved Hide resolved
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2024
tbu- added 2 commits June 12, 2024 17:51
Addresses rust-lang#124636 (comment).

I think that the diff display regresses a little, because it's no longer
showing the `+` to show where the `unsafe {}` is added. I think it's
still fine.
@tbu-
Copy link
Contributor Author

tbu- commented Jun 12, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 12, 2024
@fee1-dead
Copy link
Member

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Jun 12, 2024

📌 Commit 4f5fb31 has been approved by fee1-dead

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 Jun 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125869 (Add `target_env = "p1"` to the `wasm32-wasip1` target)
 - rust-lang#126019 (Add TODO comment to unsafe env modification)
 - rust-lang#126036 (Migrate `run-make/short-ice` to `rmake`)
 - rust-lang#126276 (Detect pub structs never constructed even though they impl pub trait with assoc constants)
 - rust-lang#126282 (Ensure self-contained linker is only enabled on dev/nightly )
 - rust-lang#126317 (Avoid a bunch of booleans in favor of Result<(), ErrorGuaranteed> as that more robustly proves that an error has been emitted)
 - rust-lang#126324 (Adjust LoongArch64 data layouts for LLVM update)
 - rust-lang#126340 (Fix outdated predacates_of.rs comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 88984fe into rust-lang:master Jun 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
Rollup merge of rust-lang#126019 - tbu-:pr_unsafe_env_fixme, r=fee1-dead

Add TODO comment to unsafe env modification

Addresses rust-lang#124636 (comment).

I think that the diff display regresses a little, because it's no longer showing the `+` to show where the `unsafe {}` is added. I think it's still fine.

Tracking:
- rust-lang#124866

r? `@RalfJung`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants