search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.8k stars 12.66k forks source link

Remove `NtIdent` hack for regressed crates #74616

Open Aaron1011 opened 4 years ago

Aaron1011 commented 4 years ago

What is this issue?

If you're a crate author who's been linked here, this issue tracks removing a backwards-compatibility hack in Rust.

Rust has a longstanding issue https://github.com/rust-lang/rust/issues/43081, which causes procedural macros to lose location and hygiene information (known as a "Span") under certain circumstances. Recently, pull request https://github.com/rust-lang/rust/pull/73084 was merged, which makes progress towards resolving https://github.com/rust-lang/rust/issues/43081.

Unfortunately, older versions of certain procedural macro crates (such as proc-macro-hack v0.5.15 and js-sys v0.3.39) cannot handle the changes in input caused by the Rust bugfix. To allow these crates to continue to compile, a backward-compatibility hack was added to adjust the input passed to proc-macro-hack and js-sys specifically.

Eventually, we would like to remove this backwards-compatibility hack, since the compiler should not have hard-coded exceptions for certain crates. However, removing this hack will break any crates that depend on affected versions of proc-macro-hack or js-sys.

To ensure that your crate continues to work, you'll want to ensure that your Cargo.lock references proc-macro-hack v0.5.16 or above, and js-sys v0.3.40 or above. This can be done by running cargo update -p proc-macro-hack and cargo update -p js-sys. If you maintain a library crate (without a Cargo.lock, no action is needed on your part).

Internal compiler details

In https://github.com/rust-lang/rust/pull/73084#issuecomment-652613950, I added a hack to change the behavior of NtIdents passed to certain proc-macros. This was done by special-casing certain identifiers, and should be eventually be removed in favor of a proper solution.

If we decide to always wrap single identifiers in None-delimited groups, then we will need to wait until enough of the ecosystem has bumped proc-macro-hack and wasm-bindgen, to avoid breaking a large number of crates.

Crater run: https://crater-reports.s3.amazonaws.com/pr-73084-1/index.html Triage: https://hackmd.io/O7icbSylRP6uVZyAQ9EDeA

petrochenkov commented 4 years ago

Tests that need to be removed together with the hack are in src/test/ui/proc-macro/group-compat-hack.

Aaron1011 commented 4 years ago

The hack is here: https://github.com/rust-lang/rust/blob/03017003c77d782cf7ed841a05d7c628a9b93f25/src/librustc_ast/token.rs#L814-L838

Aaron1011 commented 4 years ago

I don't know why I thought that my PR fixed this issue...

bmisiak commented 4 years ago

To ensure that your crate continues to work, you'll want to ensure that your Cargo.lock references proc-macro-hack v0.5.16 or above, and js-sys v0.3.40 or above. This can be done by running cargo update -p proc-macro-hack and cargo update -p js-sys. If you maintain a library crates (without a Cargo.lock, no action is needed on your part).

Should we communicate this to users when compiling code with older versions? Give them a special deprecation warning perhaps and a hint like this, to speed things up?

Aaron1011 commented 4 years ago

PR https://github.com/rust-lang/rust/pull/75534 will give us the first part of the needed infrastructure to do this. Right now, our only option would be to ignore cap-lints and emit the full warning for upstream dependencies, which would break anyone who denys warnings.