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

Tracking issue for deprecation lint legacy_derive_helpers #79202

Open
petrochenkov opened this issue Nov 19, 2020 · 1 comment
Open

Tracking issue for deprecation lint legacy_derive_helpers #79202

petrochenkov opened this issue Nov 19, 2020 · 1 comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 19, 2020

This is a tracking issue for deprecating code like this:

#[trait_helper] // This helper attribute is used before it introduced by `#[derive(Trait)]` below.
#[derive(Trait)]
struct S;

Code like this works for historical reasons, but attribute expansion works in left-to-right order, so we have to use some hacks and "look into the future" to resolve trait_helper, but that future may never actually happen in cases like

#[trait_helper]
#[remove_all_derives] // removes the `#[derive(Trait)]` below
#[derive(Trait)]
struct S;

, so the resolution in this case is unreliable.

A deprecation lint for this case is added in #79078 as warn-by-default.

@camelid camelid added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Nov 20, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 7, 2021
expand/resolve: Turn `#[derive]` into a regular macro attribute

This PR turns `#[derive]` into a regular attribute macro declared in libcore and defined in `rustc_builtin_macros`, like it was previously done with other "active" attributes in rust-lang#62086, rust-lang#62735 and other PRs.
This PR is also a continuation of rust-lang#65252, rust-lang#69870 and other PRs linked from them, which layed the ground for converting `#[derive]` specifically.

`#[derive]` still asks `rustc_resolve` to resolve paths inside `derive(...)`, and `rustc_expand` gets those resolution results through some backdoor (which I'll try to address later), but otherwise `#[derive]` is treated as any other macro attributes, which simplifies the resolution-expansion infra pretty significantly.

The change has several observable effects on language and library.
Some of the language changes are **feature-gated** by [`feature(macro_attributes_in_derive_output)`](rust-lang#81119).

#### Library

- `derive` is now available through standard library as `{core,std}::prelude::v1::derive`.

#### Language

- `derive` now goes through name resolution, so it can now be renamed - `use derive as my_derive; #[my_derive(Debug)] struct S;`.
- `derive` now goes through name resolution, so this resolution can fail in corner cases. Crater found one such regression, where import `use foo as derive` goes into a cycle with `#[derive(Something)]`.
- **[feature-gated]** `#[derive]` is now expanded as any other attributes in left-to-right order. This allows to remove the restriction on other macro attributes following `#[derive]` (rust-lang/reference#566). The following macro attributes become a part of the derive's input (this is not a change, non-macro attributes following `#[derive]` were treated in the same way previously).
- `#[derive]` is now expanded as any other attributes in left-to-right order. This means two derive attributes `#[derive(Foo)] #[derive(Bar)]` are now expanded separately rather than together. It doesn't generally make difference, except for esoteric cases. For example `#[derive(Foo)]` can now produce an import bringing `Bar` into scope, but previously both `Foo` and `Bar` were required to be resolved before expanding any of them.
- **[feature-gated]** `#[derive()]` (with empty list in parentheses) actually becomes useful. For historical reasons `#[derive]` *fully configures* its input, eagerly evaluating `cfg` everywhere in its target, for example on fields.
Expansion infra doesn't do that for other attributes, but now when macro attributes attributes are allowed to be written after `#[derive]`, it means that derive can *fully configure* items for them.
    ```rust
	#[derive()]
	#[my_attr]
	struct S {
		#[cfg(FALSE)] // this field in removed by `#[derive()]` and not observed by `#[my_attr]`
		field: u8
	}
    ```
- `#[derive]` on some non-item targets is now prohibited. This was accidentally allowed as noop in the past, but was warned about since early 2018 (rust-lang#50092), despite that crater found a few such cases in unmaintained crates.
- Derive helper attributes used before their introduction are now reported with a deprecation lint. This change is long overdue (since macro modularization, rust-lang#52226 (comment)), but it was hard to do without fixing expansion order for derives. The deprecation is tracked by rust-lang#79202.
```rust
    #[trait_helper] // warning: derive helper attribute is used before it is introduced
    #[derive(Trait)]
    struct S {}
```

Crater analysis: rust-lang#79078 (comment)
peddermaster2 pushed a commit to peddermaster2/rust-analyzer that referenced this issue Feb 12, 2021
bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Feb 12, 2021
7644: Primitive completion r=jonas-schievink a=jonas-schievink

Fixes #7642

7648: fix nightly warning `legacy_derive_helpers` r=jonas-schievink a=peddermaster2

With a recent nightly (e.g. 2021-02-10) a warning comes up. This PR reorders the attributes to fix the warning.

See rust-lang/rust#79202

Co-authored-by: Jonas Schievink <[email protected]>
Co-authored-by: Peter Wischer <[email protected]>
bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Feb 12, 2021
7648: fix nightly warning `legacy_derive_helpers` r=jonas-schievink a=peddermaster2

With a recent nightly (e.g. 2021-02-10) a warning comes up. This PR reorders the attributes to fix the warning.

See rust-lang/rust#79202

Co-authored-by: Peter Wischer <[email protected]>
peddermaster2 pushed a commit to peddermaster2/rust-analyzer that referenced this issue Feb 12, 2021
peddermaster2 pushed a commit to peddermaster2/rust-analyzer that referenced this issue Feb 12, 2021
bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Feb 12, 2021
7638: libloading 0.7 r=kjeremy a=kjeremy

See https://docs.rs/libloading/0.7.0/libloading/changelog/r0_7_0/index.html

7648: fix nightly warning `legacy_derive_helpers` r=lnicola a=peddermaster2

With a recent nightly (e.g. 2021-02-10) a warning comes up. This PR reorders the attributes to fix the warning.

See rust-lang/rust#79202

Co-authored-by: kjeremy <[email protected]>
Co-authored-by: Peter Wischer <[email protected]>
jonasbb added a commit to jonasbb/serde_with that referenced this issue Feb 15, 2021
Sort inert attributes after the derive.

rust-lang/rust#79202
bors bot added a commit to jonasbb/serde_with that referenced this issue Feb 15, 2021
265: Fix nightly warning legacy_derive_helpers r=jonasbb a=jonasbb

Sort inert attributes after the derive.

rust-lang/rust#79202

bors r+

Co-authored-by: Jonas Bushart <[email protected]>
SimonSapin added a commit to servo/servo that referenced this issue Feb 25, 2021
This does not (yet) upgrade ./rust-toolchain

The warnings:

* dead_code "field is never read"
* redundant_semicolons "unnecessary trailing semicolon"
* non_fmt_panic "panic message is not a string literal, this is no longer accepted in Rust 2021"
* unstable_name_collisions "a method with this name may be added to the standard library in the future"
* legacy_derive_helpers "derive helper attribute is used before it is introduced" rust-lang/rust#79202
SimonSapin added a commit to servo/servo that referenced this issue Feb 25, 2021
This does not (yet) upgrade ./rust-toolchain

The warnings:

* dead_code "field is never read"
* redundant_semicolons "unnecessary trailing semicolon"
* non_fmt_panic "panic message is not a string literal, this is no longer accepted in Rust 2021"
* unstable_name_collisions "a method with this name may be added to the standard library in the future"
* legacy_derive_helpers "derive helper attribute is used before it is introduced" rust-lang/rust#79202
SimonSapin added a commit to servo/servo that referenced this issue Feb 25, 2021
This does not (yet) upgrade ./rust-toolchain

The warnings:

* dead_code "field is never read"
* redundant_semicolons "unnecessary trailing semicolon"
* non_fmt_panic "panic message is not a string literal, this is no longer accepted in Rust 2021"
* unstable_name_collisions "a method with this name may be added to the standard library in the future"
* legacy_derive_helpers "derive helper attribute is used before it is introduced" rust-lang/rust#79202
SimonSapin added a commit to servo/servo that referenced this issue Feb 25, 2021
This does not (yet) upgrade ./rust-toolchain

The warnings:

* dead_code "field is never read"
* redundant_semicolons "unnecessary trailing semicolon"
* non_fmt_panic "panic message is not a string literal, this is no longer accepted in Rust 2021"
* unstable_name_collisions "a method with this name may be added to the standard library in the future"
* legacy_derive_helpers "derive helper attribute is used before it is introduced" rust-lang/rust#79202
Aaron1011 added a commit to Aaron1011/wgpu that referenced this issue Feb 25, 2021
bors bot added a commit to gfx-rs/wgpu that referenced this issue Feb 26, 2021
1237: Move `#[error]` attributes after the corresponding `#[derive]` r=kvark a=Aaron1011

**Connections**
See rust-lang/rust#79202

**Description**
Fixes Nightly future-incompat warnings

**Testing**
There are no behavior changes intended.

Co-authored-by: Aaron Hill <[email protected]>
Swatinem added a commit to Swatinem/rslint that referenced this issue Mar 14, 2021
In particular, this takes care of rust-lang/rust#79202
as well as some clippy nightly suggestions.

There is a new very noisy https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
but I didn’t take care of that one.
RDambrosio016 pushed a commit to rslint/rslint that referenced this issue Mar 15, 2021
In particular, this takes care of rust-lang/rust#79202
as well as some clippy nightly suggestions.

There is a new very noisy https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
but I didn’t take care of that one.
pierwill added a commit to influxdata/flux that referenced this issue Apr 1, 2021
On current nightly, this fixes a warning:

    error: derive helper attribute is used before it is introduced

It goes on to say

    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #79202 <rust-lang/rust#79202>

This change gets ahead of the issue.
pilocchii pushed a commit to pilocchii/rsgen-avro that referenced this issue Jul 1, 2021
lerouxrgd pushed a commit to lerouxrgd/rsgen-avro that referenced this issue Jul 1, 2021
These will become hard errors in the future
See <rust-lang/rust#79202>

Co-authored-by: Alyssa Ingersoll <[email protected]>
lu-zero added a commit to lu-zero/cargo-ebuild that referenced this issue Jul 29, 2021
AltF02 added a commit to AltF02/cargo-ebuild that referenced this issue Jul 30, 2021
AltF02 added a commit to AltF02/cargo-ebuild that referenced this issue Jul 30, 2021
lu-zero added a commit to lu-zero/cargo-ebuild that referenced this issue Aug 2, 2021
gentoo-bot pushed a commit to gentoo/cargo-ebuild that referenced this issue Aug 2, 2021
tkmcmaster added a commit to FamilySearch/pewpew that referenced this issue Sep 2, 2021
tkmcmaster added a commit to FamilySearch/pewpew that referenced this issue Sep 3, 2021
* Updated results viewer dependencies and fixed sed command

* Added sed fix to update-guild.yml

* Added tests for the hdr-histogram-wasm

* Updated dependencies for config-wasm tests

* Updated the comment to show that wasm-pack v0.10.0 works

* Renamed tests directory to match config directory

* Renamed config_wasm to config-wasm to match hdr-histogram-wasm

* Updated the mdbook and wasm-pack versions in the Dockerfile

* Updated the workflows

- Updated the mdbook and wasm-pack versions
- Updated pr and release to build both config-wasm and hdr-histogram-wasm and run tests

* Updated the lock file to the latest dependencies

* Fixed warning in latest version of Rust

rust-lang/rust#79202

* Changed the config-wasm and hdr-histogram-wasm to be scoped '@fs'

* extraneous spaces from empty lines

* Fixed the names in the Javascript PR workflow

* Initial fixes for clippy (failing tests)

* Updated babel version
github-merge-queue bot pushed a commit to smithy-lang/smithy-rs that referenced this issue Apr 19, 2023
## Motivation and Context
Follow-up to #2434


## Description
This provides a way to fix a compiler warning. Attributes created using
RustMetadata.additionalAttributes may trigger compiler warnings
(rust-lang/rust#79202) such as

```
warning: derive helper attribute is used before it is introduced
    --> src/model.rs:7674:3
     |
7674 | #[serde(tag = "_type", content = "_content")]
     |   ^^^^^
7675 | #[derive(
7676 |     serde::Deserialize,
     |     ------------------ the attribute is introduced here
     |
     = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     = note: for more information, see issue #79202 <rust-lang/rust#79202>

```

## Testing
Added a unit test to validate the sort order is applied correctly to
Attributes with isDeriveHelper = true.

---------

Co-authored-by: david-perez <[email protected]>
unexge pushed a commit to smithy-lang/smithy-rs that referenced this issue Apr 24, 2023
## Motivation and Context
Follow-up to #2434


## Description
This provides a way to fix a compiler warning. Attributes created using
RustMetadata.additionalAttributes may trigger compiler warnings
(rust-lang/rust#79202) such as

```
warning: derive helper attribute is used before it is introduced
    --> src/model.rs:7674:3
     |
7674 | #[serde(tag = "_type", content = "_content")]
     |   ^^^^^
7675 | #[derive(
7676 |     serde::Deserialize,
     |     ------------------ the attribute is introduced here
     |
     = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     = note: for more information, see issue #79202 <rust-lang/rust#79202>

```

## Testing
Added a unit test to validate the sort order is applied correctly to
Attributes with isDeriveHelper = true.

---------

Co-authored-by: david-perez <[email protected]>
rcoh pushed a commit to smithy-lang/smithy-rs that referenced this issue Apr 24, 2023
## Motivation and Context
Follow-up to #2434


## Description
This provides a way to fix a compiler warning. Attributes created using
RustMetadata.additionalAttributes may trigger compiler warnings
(rust-lang/rust#79202) such as

```
warning: derive helper attribute is used before it is introduced
    --> src/model.rs:7674:3
     |
7674 | #[serde(tag = "_type", content = "_content")]
     |   ^^^^^
7675 | #[derive(
7676 |     serde::Deserialize,
     |     ------------------ the attribute is introduced here
     |
     = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     = note: for more information, see issue #79202 <rust-lang/rust#79202>

```

## Testing
Added a unit test to validate the sort order is applied correctly to
Attributes with isDeriveHelper = true.

---------

Co-authored-by: david-perez <[email protected]>
@oli-obk
Copy link
Contributor

oli-obk commented Jan 5, 2024

This issue hasn't been referred to in over half a year, and only sparingly before that. We may want to try out making this a hard error

@wesleywiser wesleywiser added the S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR label Jan 5, 2024
kxxt added a commit to kxxt/filterable-enum that referenced this issue Apr 26, 2024
cbgbt pushed a commit to cbgbt/bottlerocket-settings-sdk that referenced this issue Jun 17, 2024
Rust 1.52 added a legacy_derive_helpers warning (soon to be an error) that
yells if you use an attribute macro before the derive macro that introduces it.
We should always put derive macros at the start of the list to avoid this.

Reference: rust-lang/rust#79202
azishio added a commit to azishio/geo-tileset-rs that referenced this issue Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants