Skip to content

Conversation

@spastorino
Copy link
Member

@spastorino spastorino commented Jun 20, 2024

@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 20, 2024
@rust-log-analyzer

This comment has been minimized.

@spastorino spastorino force-pushed the avoid-safe-outside-unsafe-blocks branch from 8711347 to 8cef2ba Compare June 20, 2024 19:31
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.

This doesn't gate safe on cfg'd out code, so this still passes:

#[cfg(FALSE)]
safe fn foo() {}

I'm not sure if that was intentional, since I thought you intended to put this up as an alternative to #126757. If that's not the case, then you can ignore this.

This also doesn't consider other positions where safety may show up, like:

  • Impl items and trait items
  • Function pointers

@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 20, 2024
@compiler-errors
Copy link
Member

edited my review comment a bit since i'm not sure what the relationship between #126757 and this PR is supposed to be.

@spastorino spastorino force-pushed the avoid-safe-outside-unsafe-blocks branch from 8cef2ba to edf32cf Compare June 20, 2024 21:05
@spastorino spastorino force-pushed the avoid-safe-outside-unsafe-blocks branch 2 times, most recently from df6aa7a to e70d112 Compare June 20, 2024 21:28
@spastorino spastorino force-pushed the avoid-safe-outside-unsafe-blocks branch from e70d112 to 3d6d9e2 Compare June 20, 2024 21:39
@traviscross traviscross changed the title Do not allow safe usafe on static and fn items Do not allow safe/unsafe on static and fn items Jun 21, 2024
@traviscross traviscross added the F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]` label Jun 21, 2024
@spastorino
Copy link
Member Author

@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 21, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

📌 Commit 3d6d9e2 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 Jun 21, 2024
@spastorino spastorino force-pushed the avoid-safe-outside-unsafe-blocks branch from 3d6d9e2 to 22831ed Compare June 21, 2024 12:12
@spastorino
Copy link
Member Author

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

📌 Commit 22831ed 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2024
@ytmimi
Copy link
Contributor

ytmimi commented Jun 21, 2024

@compiler-errors @spastorino is this going to cause issues with https://github.com/rust-lang/rustfmt/pull/6204/files#r1648395270? Specifically I'm wondering about the test case that uses safe on the static TEST1 item.

@compiler-errors
Copy link
Member

No -- this is validation that only happens when you try to compile the code.

@ytmimi
Copy link
Contributor

ytmimi commented Jun 21, 2024

Awesome. Thanks for the quick response. Just wanted to make sure that this wouldn't cause issues for the upcoming sync

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

⌛ Testing commit 22831ed with merge 9cf721a...

@bors
Copy link
Collaborator

bors commented Jun 21, 2024

💔 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 Jun 21, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
  SDKROOT: /Applications/Xcode_14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk
  AR: ar
##[endgroup]
curl: (28) Operation too slow. Less than 100 bytes/sec transferred the last 5 seconds
Error: Failure while executing; `/usr/bin/env /usr/local/Homebrew/Library/Homebrew/shims/shared/curl --disable --cookie /dev/null --globoff --user-agent Homebrew/4.3.5\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.7\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --remote-time --output /Users/runner/Library/Caches/Homebrew/api/cask.jws.json --location --disable --cookie /dev/null --globoff --show-error --user-agent Homebrew/4.3.5\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.7\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --compressed --speed-limit 100 --speed-time 5 https://formulae.brew.sh/api/cask.jws.json` exited with 28. Here's the output:


##[error]Failure while executing; `/usr/bin/env /usr/local/Homebrew/Library/Homebrew/shims/shared/curl --disable --cookie /dev/null --globoff --user-agent Homebrew/4.3.5\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.7\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --remote-time --output /Users/runner/Library/Caches/Homebrew/api/cask.jws.json --location --disable --cookie /dev/null --globoff --show-error --user-agent Homebrew/4.3.5\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.7\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --compressed --speed-limit 100 --speed-time 5 https://formulae.brew.sh/api/cask.jws.json` exited with 28. Here's the output:
##[error]Process completed with exit code 1.
Post job cleanup.
[command]/usr/local/bin/git version
git version 2.45.2

@compiler-errors
Copy link
Member

@bors retry

@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 21, 2024
@bors
Copy link
Collaborator

bors commented Jun 22, 2024

⌛ Testing commit 22831ed with merge fcae626...

@bors
Copy link
Collaborator

bors commented Jun 22, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 22, 2024
@bors bors merged commit fcae626 into rust-lang:master Jun 22, 2024
@rustbot rustbot added this to the 1.81.0 milestone Jun 22, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fcae626): 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 (primary -4.3%)

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)
-4.3% [-4.3%, -4.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.3% [-4.3%, -4.3%] 1

Cycles

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

Binary size

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

Bootstrap: 697.477s -> 693.147s (-0.62%)
Artifact size: 326.87 MiB -> 326.88 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]` 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.