Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON. by obi1kenobi · Pull Request #143555 · rust-lang/rust

obi1kenobi · 2025-07-07T01:06:39Z

Fixes #142655 by explicitly checking whether functions are safe but using #[target_feature], instead of relying on the FnHeader::is_unsafe() method which considers such functions unsafe.

I don't believe this merits a bump of the rustdoc JSON FORMAT_VERSION constant, since the format is unchanged and this is just a small bugfix.

r? aDotInTheVoid

rustbot · 2025-07-07T01:06:45Z

These commits modify tests/rustdoc-json.
rustdoc-json is a public (but unstable) interface.

Please ensure that if you've changed the output:

It's intentional.
The FORMAT_VERSION in src/librustdoc-json-types is bumped if necessary.

cc @aDotInTheVoid, @obi1kenobi

obi1kenobi · 2025-07-07T01:13:22Z

While reading up on the overall context, I noticed that @oli-obk had considered changing the Safety enum to express the #[target_feature] complexity in #134317 and appears to have decided in favor of a smaller change in #134353.

I'm not qualified to decide whether refactoring Safety or FnHeader to fully capture #[target_feature]'s complexity is worth it, nor what the best way to do it is. I just wanted to raise the fact that we now know the fix from #134353 isn't entirely without its own "easy to get wrong" aspects, so it may be worth another look.

oli-obk · 2025-07-07T07:36:01Z

src/librustdoc/json/conversions.rs

+        let is_unsafe =
+            header.is_unsafe() && !matches!(header.safety, HeaderSafety::SafeTargetFeatures);


Make this an exhaustive match on header.safety (not not header.safety()!).

Tho long term I think we'll want to change the rustdoc::json::FunctionHeader to just fully have the header safety information. Even normal rustdoc rendering should probably distinguish between safe functions and safe-in-some-contexts functions and have appropriate hints somewhere.

Sounds good, thanks Oli!

aDotInTheVoid · 2025-07-07T20:50:21Z

src/librustdoc/json/conversions.rs

+        // The type system's internal implementation details consider
+        // safe functions with the `#[target_feature]` attribute as analogous to unsafe functions.
+        // `header.is_unsafe()` returns `true` for them. But that's not the right decision
+        // for rustdoc, so we specifically exclude that case.


NIT: It might be nice to link back to #142655 here

Done inside the exhaustive match's HeaderSafety::SafeTargetFeatures arm.

aDotInTheVoid · 2025-07-07T20:51:49Z

Thanks!

r=me with oli's match thing changed. (You don't need to handle my nit).

@bors delegate+

bors · 2025-07-07T20:51:53Z

✌️ @obi1kenobi, you can now approve this pull request!

If @aDotInTheVoid told you to "r=me" after making some further change, please make that change, then do @bors r=@aDotInTheVoid

obi1kenobi · 2025-07-08T03:29:52Z

@bors r=aDotInTheVoid

bors · 2025-07-08T03:29:55Z

📌 Commit 7170412 has been approved by aDotInTheVoid

It is now in the queue for this repository.

…nsafe-rustdoc-json, r=aDotInTheVoid Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON. Fixes rust-lang#142655 by explicitly checking whether functions are safe but using `#[target_feature]`, instead of relying on the `FnHeader::is_unsafe()` method which considers such functions unsafe. I don't believe this merits a bump of the rustdoc JSON `FORMAT_VERSION` constant, since the format is unchanged and this is just a small bugfix. r? aDotInTheVoid

Rollup of 6 pull requests Successful merges: - #134628 (Make `Default` const and add some `const Default` impls) - #143555 (Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.) - #143600 (Update intro blurb in `wasm32-wasip1` docs) - #143603 (Clarify the meaning of `AttributeOrder::KeepFirst` and `AttributeOrder::KeepLast`) - #143620 (fix: Remove newline from multiple crate versions note) - #143622 (Add target maintainer information for mips64-unknown-linux-muslabi64) r? `@ghost` `@rustbot` modify labels: rollup

…nsafe-rustdoc-json, r=aDotInTheVoid Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON. Fixes rust-lang#142655 by explicitly checking whether functions are safe but using `#[target_feature]`, instead of relying on the `FnHeader::is_unsafe()` method which considers such functions unsafe. I don't believe this merits a bump of the rustdoc JSON `FORMAT_VERSION` constant, since the format is unchanged and this is just a small bugfix. r? aDotInTheVoid

Rollup of 8 pull requests Successful merges: - #143402 (Port several linking (linkage?) related attributes the new attribute system ) - #143555 (Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.) - #143593 (Port #[rustc_dummy]) - #143600 (Update intro blurb in `wasm32-wasip1` docs) - #143603 (Clarify the meaning of `AttributeOrder::KeepFirst` and `AttributeOrder::KeepLast`) - #143606 (configure.py: Write last key in each section) - #143620 (fix: Remove newline from multiple crate versions note) - #143622 (Add target maintainer information for mips64-unknown-linux-muslabi64) Failed merges: - #143403 (Port several trait/coherence-related attributes the new attribute system) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 7 pull requests Successful merges: - #143402 (Port several linking (linkage?) related attributes the new attribute system ) - #143555 (Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.) - #143593 (Port #[rustc_dummy]) - #143600 (Update intro blurb in `wasm32-wasip1` docs) - #143603 (Clarify the meaning of `AttributeOrder::KeepFirst` and `AttributeOrder::KeepLast`) - #143620 (fix: Remove newline from multiple crate versions note) - #143622 (Add target maintainer information for mips64-unknown-linux-muslabi64) Failed merges: - #143403 (Port several trait/coherence-related attributes the new attribute system) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #143555 - obi1kenobi:pg/target-feature-not-unsafe-rustdoc-json, r=aDotInTheVoid Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON. Fixes #142655 by explicitly checking whether functions are safe but using `#[target_feature]`, instead of relying on the `FnHeader::is_unsafe()` method which considers such functions unsafe. I don't believe this merits a bump of the rustdoc JSON `FORMAT_VERSION` constant, since the format is unchanged and this is just a small bugfix. r? aDotInTheVoid

rustbot assigned aDotInTheVoid Jul 7, 2025

rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 7, 2025

oli-obk reviewed Jul 7, 2025

View reviewed changes

aDotInTheVoid reviewed Jul 7, 2025

View reviewed changes

Don't mark #[target_feature] safe fns as unsafe in rustdoc JSON.

7170412

obi1kenobi force-pushed the pg/target-feature-not-unsafe-rustdoc-json branch from 239f472 to 7170412 Compare July 8, 2025 02:03

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 Jul 8, 2025

matthiaskrgr mentioned this pull request Jul 8, 2025

Rollup of 6 pull requests #143627

Closed

matthiaskrgr mentioned this pull request Jul 8, 2025

Rollup of 8 pull requests #143643

Closed

matthiaskrgr mentioned this pull request Jul 8, 2025

Rollup of 7 pull requests #143645

Merged

bors merged commit cbf7f80 into rust-lang:master Jul 8, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.#143555

Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.#143555
bors merged 1 commit intorust-lang:masterfrom
obi1kenobi:pg/target-feature-not-unsafe-rustdoc-json

obi1kenobi commented Jul 7, 2025

Uh oh!

rustbot commented Jul 7, 2025

Uh oh!

obi1kenobi commented Jul 7, 2025

Uh oh!

oli-obk Jul 7, 2025

Uh oh!

obi1kenobi Jul 8, 2025

Uh oh!

aDotInTheVoid Jul 7, 2025

Uh oh!

obi1kenobi Jul 8, 2025

Uh oh!

aDotInTheVoid commented Jul 7, 2025

Uh oh!

bors commented Jul 7, 2025

Uh oh!

obi1kenobi commented Jul 8, 2025

Uh oh!

bors commented Jul 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		let is_unsafe =
		header.is_unsafe() && !matches!(header.safety, HeaderSafety::SafeTargetFeatures);

Uh oh!

Conversation

obi1kenobi commented Jul 7, 2025

Uh oh!

rustbot commented Jul 7, 2025

Uh oh!

obi1kenobi commented Jul 7, 2025

Uh oh!

oli-obk Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

obi1kenobi Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

aDotInTheVoid Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

obi1kenobi Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

aDotInTheVoid commented Jul 7, 2025

Uh oh!

bors commented Jul 7, 2025

Uh oh!

obi1kenobi commented Jul 8, 2025

Uh oh!

bors commented Jul 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants