Prepare removal of `auto trait` syntax in favor of new attribute `#[rustc_auto_trait]` #116126

fmease · 2023-09-24T22:14:18Z

Continue to parse auto trait but reject it after macro expansion
- Necessary for bootstrapping
- Should lessen ecosystem impact (assuming third-party crates gate auto traits behind a Cargo feature flag)
- Full removal of the syntax can happen earliest after the next beta bump that occurs once this PR has been merged
Introduce the internal attribute #[rustc_auto_trait] as a replacement for auto trait
Remove the feature auto_traits in favor of the internal feature rustc_attrs
Update the world

It's unclear if this PR is blocked on an RFC due to backward compatibility concerns and due to potential ecosystem churn as potentially several third-party libraries have started to rely on this unstable but essentially internal feature.
Investigations needed (e.g., crater run). Reportedly, pyo3 relies on auto traits behind a Cargo feature flag.

r? @ghost

Noratrieb · 2023-09-25T04:55:00Z

I don't think we need an RFC to change the syntax. Libraries that use this know what they're doing and can live with it.

Additionally, can you remove the is_auto from the AST?

Noratrieb

I see, it's still in the AST to emit an error, makes sense. Ideally we can remove that too at some point but for now it's fine.

I don't think we should bother cratering this and fixing people's code. The error message is pretty clear so people should know what to do and it's a nightly feature that was always pretty experimental unlike say, box syntax. But even for box syntax we didn't care about fixing anyone.

compiler/rustc_ast/src/ast.rs

Noratrieb · 2023-09-25T05:05:28Z

compiler/rustc_feature/src/builtin_attrs.rs

@@ -551,6 +551,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
        may_dangle, Normal, template!(Word), WarnFollowing, dropck_eyepatch,
        "`may_dangle` has unstable semantics and may be removed in the future",
    ),
+    gated!(
+        rustc_auto_trait, Normal, template!(Word), WarnFollowing, auto_traits,


This should already be gated with rustc_attrs, I don't see the point in having an extra gate. It should be moved to the other rustc attrs.

+1 on removing auto_traits and making #[rustc_auto_trait] part of rustc_attrs. I didn't do that since people in the Zulip topic were proposing marking auto_traits internal instead of removing it.

compiler/rustc_hir/src/hir.rs

Noratrieb · 2023-09-25T05:07:34Z

compiler/rustc_hir_analysis/src/collect.rs

-        hir::ItemKind::Trait(is_auto, unsafety, .., items) => {
-            (is_auto == hir::IsAuto::Yes, unsafety, items)
+        hir::ItemKind::Trait(unsafety, .., items) => {
+            (tcx.has_attr(def_id, sym::rustc_auto_trait), unsafety, items)


could you move the auto check next to the conductive check?

fmease · 2023-09-25T11:49:56Z

Additionally, can you remove the is_auto from the AST?

I see, it's still in the AST to emit an error, makes sense. Ideally we can remove that too at some point but for now it's fine.

This is a nice side effect, the actual reason being the bootstrap process: To build the stage1 compiler artifacts, the beta compiler compiles core (etc.) beforehand. Therefore we can't remove the syntax just yet (and modify the parser and AST), not until the next beta bump that occurs after this PR has been merged. You can see my heavy use of cfg(bootstrap) in library/ and compiler/rustc_data_structures/ of the form:

#[cfg(bootstrap)]
auto trait Trait {}

#[cfg(not(bootstrap))]
#[rustc_auto_trait]
trait Trait {}

Initially I planned on opening two PRs simultaneously, this one and one that updates the parser and the AST (blocked on beta bump) but since I wasn't sure how long the grace period should be and if this PR here gets accepted at all, I didn't go through with it in the end.

fmease · 2023-09-27T12:52:03Z

src/doc/unstable-book/src/language-features/auto-traits.md

It's a shame that this beautifully written chapter has to be removed. I figure the contents could be moved to the rustc dev guide?

it definitely shouldn't be deleted from reality^^

Noratrieb · 2023-09-27T19:05:37Z

compiler/rustc_ast_passes/src/feature_gate.rs

-                    i.span,
-                    "auto traits are experimental and possibly buggy"
-                );
+                // FIXME: Consider using a structured suggestion.


not having one is fine imo, but you can add one if you want to

Noratrieb · 2023-09-27T19:09:42Z

compiler/rustc_error_codes/src/error_codes/E0321.md

 or enum type.

 Erroneous code example:

 ```compile_fail,E0321
-#![feature(auto_traits)]


should this use negative_impls?

I guess I can add it but it doesn't seem mandatory. Apparently, stderr is allowed to contain more errors than listed after compile_fail. Several other error code code blocks also lack #![feature].

yeah, it doesn't seem mandatory but it's nice to have it

Noratrieb

I took a closer look and have a few more comments. But the more I look at it, the more I like it, I think this is a great change.

Noratrieb · 2023-09-27T19:10:43Z

compiler/rustc_feature/src/removed.rs

@@ -50,6 +50,9 @@ declare_features! (
    (removed, allocator, "1.0.0", None, None, None),
    /// Allows a test to fail without failing the whole suite.
    (removed, allow_fail, "1.19.0", Some(46488), None, Some("removed due to no clear use cases")),
+    /// Allows features specific to auto traits.
+    /// Renamed from `optin_builtin_traits`.
+    (removed, auto_traits, "1.50.0", Some(13231), None, Some("removed in favor of `#[feature(rustc_attrs)]`")),


instead of the features, can you show the attribute directly here?

Noratrieb · 2023-09-27T19:15:21Z

src/doc/unstable-book/src/language-features/auto-traits.md

@@ -1,106 +0,0 @@
-# `auto_traits`


leaving a comment here to remember to copy these docs to somewhere else. maybe we should have a subdirectory about rustc_attrs in the unstable book?

src/librustdoc/clean/mod.rs

Noratrieb · 2023-09-27T19:19:25Z

tests/ui/macros/stringify.rs

    assert_eq!(
        stringify_item!(
-            pub unsafe auto trait Send {}
+            pub unsafe trait Send {}
        ),
-        "pub unsafe auto trait Send {}",
+        "pub unsafe trait Send {}",
    );


can you make the one below pub and delete this assert?

Noratrieb · 2023-09-27T19:20:14Z

tests/ui/parser/obsolete-auto-trait-syntax.rs

@@ -0,0 +1,14 @@
+trait Trait1 {}


can you make this a revision test to make this fully independent of recovery, making sure that it passes for the #[cfg(not_enabled)] case?

fmease · 2023-09-29T14:40:59Z

Nominating this for T-lang discussion. If it's possible and if I can make it, I will attend the meeting.
The details on this change (motivation, drawbacks, etc.) can be found here:
https://hackmd.io/@fmease/rustc_auto_trait (Zulip topic).

bors · 2023-10-03T14:56:21Z

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

fmease · 2023-10-03T20:39:18Z

Closing as per T-lang meeting 2023-10-03.

The breakage is not worth it given the number of large crates using it like PyO3. It's even unclear if auto_traits should be marked internal after all. Unmaintained could be a better category. We might want to reuse the current auto trait syntax but change the semantics at some point to be less footgun-y.

Noratrieb · 2023-10-04T04:20:13Z

this is a nightly feature? why are we concerned about breakage like that?

Noratrieb · 2023-10-04T05:34:27Z

I see, it's mostly about the valid use cases: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Removal.20of.20.60auto.20trait.60.20syntax

That's understandable.

fmease · 2023-10-06T00:09:26Z

Right, sorry that that wasn't clear. I wrote that summary right after the meeting without checking if it made any sense.

Personally speaking I'm still an opponent of auto traits as currently designed and implemented. I can see how vital they are for modelling certain things and that they find great use in well-known crates but I wish the feature would be more constrained and controlled.

Thanks for the reviews on this PR, @Nilstrieb, regardless of the final outcome 🙏.

This comment has been minimized.

Sign in to view

fmease force-pushed the prep-auto-trait-syntax-removal branch from 6fcd0f2 to 2a9e6c5 Compare September 24, 2023 23:27

This comment has been minimized.

Sign in to view

fmease force-pushed the prep-auto-trait-syntax-removal branch from 2a9e6c5 to 63d06b2 Compare September 24, 2023 23:53

This comment has been minimized.

Sign in to view

fmease force-pushed the prep-auto-trait-syntax-removal branch 2 times, most recently from b32107f to dc9c8d5 Compare September 25, 2023 01:24

Noratrieb reviewed Sep 25, 2023

View reviewed changes

fmease changed the title ~~[blocked] Prepare removal of auto trait syntax in favor of new attribute #[rustc_auto_trait]~~ Prepare removal of auto trait syntax in favor of new attribute #[rustc_auto_trait] Sep 25, 2023

fmease force-pushed the prep-auto-trait-syntax-removal branch from dc9c8d5 to 855e006 Compare September 27, 2023 12:50

fmease commented Sep 27, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

fmease force-pushed the prep-auto-trait-syntax-removal branch from 855e006 to 81cd7b7 Compare September 27, 2023 13:24

Noratrieb reviewed Sep 27, 2023

View reviewed changes

Noratrieb self-assigned this Sep 27, 2023

mejrs mentioned this pull request Sep 28, 2023

Don't let auto trait syntax bleed through on stable Rust PyO3/pyo3#3478

Merged

fmease force-pushed the prep-auto-trait-syntax-removal branch from 81cd7b7 to c1a8109 Compare September 29, 2023 14:24

fmease force-pushed the prep-auto-trait-syntax-removal branch from c1a8109 to 21edda7 Compare September 29, 2023 14:31

fmease added the I-lang-nominated Nominated for discussion during a lang team meeting. label Sep 29, 2023

fmease mentioned this pull request Sep 29, 2023

rustdoc: don't render the auto of auto traits #116268

Open

fmease force-pushed the prep-auto-trait-syntax-removal branch from 21edda7 to ef3695d Compare October 3, 2023 17:26

This comment has been minimized.

Sign in to view

fmease added 7 commits October 3, 2023 19:37

Introduce #[rustc_auto_trait]

161b5e8

Remove IsAuto from hir::ItemKind::Trait

49f9a46

Update rustdoc to support #[rustc_auto_trait]

cf4c13c

Update tests to use #[rustc_auto_trait]

79fd1b3

Update Clippy to support #[rustc_auto_trait]

54706c0

Update the compiler itself to use #[rustc_auto_trait]

da400d2

Unstable Book: Remove chapter about feature auto_traits

c962db9

fmease force-pushed the prep-auto-trait-syntax-removal branch from ef3695d to c962db9 Compare October 3, 2023 17:38

fmease closed this Oct 3, 2023

fmease deleted the prep-auto-trait-syntax-removal branch October 3, 2023 20:39

fmease removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare removal of `auto trait` syntax in favor of new attribute `#[rustc_auto_trait]` #116126

Prepare removal of `auto trait` syntax in favor of new attribute `#[rustc_auto_trait]` #116126

fmease commented Sep 24, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Noratrieb commented Sep 25, 2023

Noratrieb left a comment

Noratrieb Sep 25, 2023

fmease Sep 25, 2023

Noratrieb Sep 25, 2023

fmease commented Sep 25, 2023 •

edited

Loading

fmease Sep 27, 2023 •

edited

Loading

Noratrieb Sep 27, 2023

This comment has been minimized.

Noratrieb Sep 27, 2023

Noratrieb Sep 27, 2023

fmease Sep 29, 2023

Noratrieb Sep 29, 2023

fmease Oct 3, 2023

Noratrieb left a comment

Noratrieb Sep 27, 2023

Noratrieb Sep 27, 2023

Noratrieb Sep 27, 2023

Noratrieb Sep 27, 2023

fmease commented Sep 29, 2023 •

edited

Loading

bors commented Oct 3, 2023

This comment has been minimized.

fmease commented Oct 3, 2023

Noratrieb commented Oct 4, 2023

Noratrieb commented Oct 4, 2023

fmease commented Oct 6, 2023 •

edited

Loading

Prepare removal of auto trait syntax in favor of new attribute #[rustc_auto_trait] #116126

Prepare removal of auto trait syntax in favor of new attribute #[rustc_auto_trait] #116126

Conversation

fmease commented Sep 24, 2023 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Noratrieb commented Sep 25, 2023

Noratrieb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease commented Sep 25, 2023 • edited Loading

fmease Sep 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Noratrieb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease commented Sep 29, 2023 • edited Loading

bors commented Oct 3, 2023

This comment has been minimized.

fmease commented Oct 3, 2023

Noratrieb commented Oct 4, 2023

Noratrieb commented Oct 4, 2023

fmease commented Oct 6, 2023 • edited Loading

Prepare removal of `auto trait` syntax in favor of new attribute `#[rustc_auto_trait]` #116126

Prepare removal of `auto trait` syntax in favor of new attribute `#[rustc_auto_trait]` #116126

fmease commented Sep 24, 2023 •

edited

Loading

fmease commented Sep 25, 2023 •

edited

Loading

fmease Sep 27, 2023 •

edited

Loading

fmease commented Sep 29, 2023 •

edited

Loading

fmease commented Oct 6, 2023 •

edited

Loading