Treat safe target_feature functions as unsafe by default [less invasive variant] #134353

oli-obk · 2024-12-15T20:52:41Z

This unblocks

Stabilize target_feature_11 #134090

As I stated in #134090 (comment) I think the previous impl was too easy to get wrong, as by default it treated safe target feature functions as safe and had to add additional checks for when they weren't. Now the logic is inverted. By default they are unsafe and you have to explicitly handle safe target feature functions.

This is the less (imo) invasive variant of #134317, as it doesn't require changing the Safety enum, so it only affects FnDefs and nothing else, as it should.

rustbot · 2024-12-15T20:52:50Z

r? @chenyukang

rustbot has assigned @chenyukang.
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 · 2024-12-15T20:52:52Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler/rustc_ast_lowering/src/delegation.rs

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

compiler/rustc_hir_typeck/src/coercion.rs

compiler/rustc_mir_build/src/check_unsafety.rs

compiler/rustc_smir/src/rustc_smir/convert/mod.rs

tests/ui/error-emitter/highlighting.svg

tests/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.stderr

tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.stderr

tests/ui/rfcs/rfc-2396-target_feature-11/trait-impl.stderr

…ler-errors Handle fndef rendering together with signature rendering Pulled out of rust-lang#134353 Changes some highlighting in type mismatch errors around fndefs

rustbot · 2024-12-16T16:02:37Z

Some changes occurred in need_type_info.rs

cc @lcnr

…ostic-cleanups, r=compiler-errors Some trait method vs impl method signature difference diagnostic cleanups Just some things I noticed while debugging a weird diagnostic in rust-lang#134353 best reviewed commit by commit

Rollup merge of rust-lang#134386 - oli-obk:some-trait-impl-diff-diagnostic-cleanups, r=compiler-errors Some trait method vs impl method signature difference diagnostic cleanups Just some things I noticed while debugging a weird diagnostic in rust-lang#134353 best reviewed commit by commit

…ler-errors Handle fndef rendering together with signature rendering Pulled out of rust-lang#134353 Changes some highlighting in type mismatch errors around fndefs

Rollup merge of rust-lang#134354 - oli-obk:push-nlrxswvpqnuk, r=compiler-errors Handle fndef rendering together with signature rendering Pulled out of rust-lang#134353 Changes some highlighting in type mismatch errors around fndefs

…-by-default, r=wesleywiser Treat safe target_feature functions as unsafe by default [less invasive variant] This unblocks * rust-lang#134090 As I stated in rust-lang#134090 (comment) I think the previous impl was too easy to get wrong, as by default it treated safe target feature functions as safe and had to add additional checks for when they weren't. Now the logic is inverted. By default they are unsafe and you have to explicitly handle safe target feature functions. This is the less (imo) invasive variant of rust-lang#134317, as it doesn't require changing the Safety enum, so it only affects FnDefs and nothing else, as it should.

…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#134216 (Enable "jump to def" feature on patterns) - rust-lang#134353 (Treat safe target_feature functions as unsafe by default [less invasive variant]) - rust-lang#134880 (Made `Path::name` only have item name rather than full name) - rust-lang#135466 (Leak check in `impossible_predicates` to avoid monomorphizing impossible instances) - rust-lang#135476 (Remove remnant of asmjs) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2025-01-14T18:24:24Z

@bors r-
#135486 (comment)

…site]

oli-obk · 2025-01-15T08:58:46Z

@bors r+ rollup=iffy

bors · 2025-01-15T08:58:49Z

📌 Commit 767d4fe has been approved by oli-obk

It is now in the queue for this repository.

oli-obk · 2025-01-15T08:58:52Z

@bors r=wesleywiser

bors · 2025-01-15T08:58:55Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: [do not merge] CI experiments #112049

bors · 2025-01-15T08:58:55Z

📌 Commit 767d4fe has been approved by wesleywiser

It is now in the queue for this repository.

bors · 2025-01-15T12:06:59Z

⌛ Testing commit 767d4fe with merge 341f603...

bors · 2025-01-15T14:56:57Z

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 341f603 to master...

rust-timer · 2025-01-15T16:50:44Z

Finished benchmarking commit (341f603): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	1

Max RSS (memory usage)

Results (primary -3.1%, secondary 0.6%)

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)	2.8%	[2.2%, 3.3%]	2
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-3.8%	[-3.8%, -3.8%]	1
All ❌✅ (primary)	-3.1%	[-3.1%, -3.1%]	1

Cycles

Results (secondary 7.2%)

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

Binary size

Results (primary 0.0%, secondary 0.2%)

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.0%	[0.0%, 0.1%]	54
Regressions ❌ (secondary)	0.2%	[0.0%, 0.4%]	21
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.1%]	54

Bootstrap: 764.012s -> 762.915s (-0.14%)
Artifact size: 326.03 MiB -> 326.09 MiB (0.02%)

…-ptr, r=oli-obk Allow coercing safe-to-call target_feature functions to safe fn pointers r? oli-obk `@oli-obk:` this is based on your PR rust-lang#134353 :-) See rust-lang#134090 (comment) for the motivation behind this change.

Rollup merge of rust-lang#135504 - veluca93:target-feature-cast-to-fn-ptr, r=oli-obk Allow coercing safe-to-call target_feature functions to safe fn pointers r? oli-obk `@oli-obk:` this is based on your PR rust-lang#134353 :-) See rust-lang#134090 (comment) for the motivation behind this change.

rustbot assigned chenyukang Dec 15, 2024

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 Dec 15, 2024

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 15, 2024

oli-obk mentioned this pull request Dec 15, 2024

Treat safe target_feature functions as unsafe by default #134317

Closed

oli-obk commented Dec 15, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk force-pushed the safe-target-feature-unsafe-by-default branch from 9bd90ce to 273c6c5 Compare December 15, 2024 21:20

oli-obk mentioned this pull request Dec 15, 2024

Handle fndef rendering together with signature rendering #134354

Merged

oli-obk force-pushed the safe-target-feature-unsafe-by-default branch from 273c6c5 to e14cb3b Compare December 16, 2024 15:51

This comment has been minimized.

Sign in to view

oli-obk force-pushed the safe-target-feature-unsafe-by-default branch from e14cb3b to 97754d0 Compare December 16, 2024 16:02

oli-obk mentioned this pull request Dec 16, 2024

Some trait method vs impl method signature difference diagnostic cleanups #134386

Merged

This comment has been minimized.

Sign in to view

oli-obk force-pushed the safe-target-feature-unsafe-by-default branch from 97754d0 to 54901b6 Compare December 17, 2024 12:39

This comment has been minimized.

Sign in to view

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 14, 2025

GuillaumeGomez mentioned this pull request Jan 14, 2025

Rollup of 5 pull requests #135486

Closed

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 Jan 14, 2025

veluca93 mentioned this pull request Jan 14, 2025

Allow coercing safe-to-call target_feature functions to safe fn pointers #135504

Merged

oli-obk added 6 commits January 15, 2025 08:58

Treat safe target_feature functions as unsafe by default

56178dd

Hide the internal unsafety of safe target_feature fn items

e1a8b0d

Render fn defs with target_features attrs with the attribute

50654e5

Render fn defs with target_features attrs with the attribute [second …

33651f4

…site]

Try to render shorthand differently

1952b87

Avoid notes that only make sense for unsafe functions

767d4fe

oli-obk force-pushed the safe-target-feature-unsafe-by-default branch from b1deae5 to 767d4fe Compare January 15, 2025 08:58

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 Jan 15, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 15, 2025

bors merged commit 341f603 into rust-lang:master Jan 15, 2025
7 checks passed

rustbot added this to the 1.86.0 milestone Jan 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Treat safe target_feature functions as unsafe by default [less invasive variant] #134353

Treat safe target_feature functions as unsafe by default [less invasive variant] #134353

oli-obk commented Dec 15, 2024 •

edited

Loading

rustbot commented Dec 15, 2024

rustbot commented Dec 15, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Dec 16, 2024

This comment has been minimized.

This comment has been minimized.

matthiaskrgr commented Jan 14, 2025

oli-obk commented Jan 15, 2025

bors commented Jan 15, 2025

oli-obk commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

rust-timer commented Jan 15, 2025

Treat safe target_feature functions as unsafe by default [less invasive variant] #134353

Treat safe target_feature functions as unsafe by default [less invasive variant] #134353

Conversation

oli-obk commented Dec 15, 2024 • edited Loading

rustbot commented Dec 15, 2024

rustbot commented Dec 15, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Dec 16, 2024

This comment has been minimized.

This comment has been minimized.

matthiaskrgr commented Jan 14, 2025

oli-obk commented Jan 15, 2025

bors commented Jan 15, 2025

oli-obk commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

bors commented Jan 15, 2025

rust-timer commented Jan 15, 2025

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

oli-obk commented Dec 15, 2024 •

edited

Loading