-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustc: Disallow machine applicability in foreign macros #52756
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
r? @oli-obk |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
4ba06a6
to
bb785c7
Compare
--> $DIR/suggestions-not-always-applicable.rs:27:5 | ||
| | ||
LL | #[foo] //~ WARN: absolute paths must start with | ||
| ^^^^^^ help: use `crate`: `crate::#[foo]` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the suggestions still show up for the users, but are just ignored by rustfix, maybe we should rather not produce any suggestions in case of future incompatibility lints from external macros.
You already have the field in place in the Diagnostic
type, this can be used to just not add any suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good t ome!
DiagnosticBuilder { | ||
handler, | ||
diagnostic, | ||
allow_machine_applicable: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be true
? I don't see any code that ever sets it to true, only to false, so the default should probably true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, this is indeed a mistake
Recent changes to lints disallowed lints from being emitted against code located in foreign macros, except for future-incompatible lints. For a future incompatible lint, however, the automatic suggestions may not be applicable! This commit updates this code path to force all applicability suggestions made to foreign macros to never be `MachineApplicable`. This should avoid rustfix actually attempting fixing these suggestions, causing non-compiling code to be produced. Closes rust-lang/cargo#5799
bb785c7
to
ca762ba
Compare
Updated! |
@bors r+ |
📌 Commit ca762ba has been approved by |
rustc: Disallow machine applicability in foreign macros Recent changes to lints disallowed lints from being emitted against code located in foreign macros, except for future-incompatible lints. For a future incompatible lint, however, the automatic suggestions may not be applicable! This commit updates this code path to force all applicability suggestions made to foreign macros to never be `MachineApplicable`. This should avoid rustfix actually attempting fixing these suggestions, causing non-compiling code to be produced. Closes rust-lang/cargo#5799
@bors: p=1 needed for the edition preview! |
⌛ Testing commit ca762ba with merge 4edb9b38498cf7364d2f2cac00ea7c0a3ba56ace... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
⌛ Testing commit ca762ba with merge a1be95df794f4ba801dd1d29acbe14067a8dd7e6... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors: retry |
rustc: Disallow machine applicability in foreign macros Recent changes to lints disallowed lints from being emitted against code located in foreign macros, except for future-incompatible lints. For a future incompatible lint, however, the automatic suggestions may not be applicable! This commit updates this code path to force all applicability suggestions made to foreign macros to never be `MachineApplicable`. This should avoid rustfix actually attempting fixing these suggestions, causing non-compiling code to be produced. Closes rust-lang/cargo#5799
Rollup of 31 pull requests Successful merges: - #52332 (dead-code lint: say "constructed" for structs) - #52340 (Document From trait implementations for OsStr, OsString, CString, and CStr) - #52628 (Cleanup some rustdoc code) - #52732 (Remove unstable and deprecated APIs) - #52745 (Update clippy to latest master) - #52756 (rustc: Disallow machine applicability in foreign macros) - #52771 (Clarify thread::park semantics) - #52810 ([NLL] Don't make "fake" match variables mutable) - #52821 (pretty print for std::collections::vecdeque) - #52822 (Fix From<LocalWaker>) - #52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper) - #52831 (remove references to AUTHORS.txt file) - #52835 (Fix Alias intra doc ICE) - #52842 (update comment) - #52846 (Add timeout to use of `curl` in bootstrap.py.) - #52851 (Make the tool_lints actually usable) - #52853 (Improve bootstrap help on stages) - #52859 (Use Vec::extend in SmallVec::extend when applicable) - #52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.) - #52867 (releases.md: fix 2 typos) - #52870 (Implement Unpin for FutureObj and LocalFutureObj) - #52876 (run-pass/const-endianness: negate before to_le()) - #52878 (Fix wrong issue number in the test name) - #52883 (Include lifetime in mutability suggestion in NLL messages) - #52904 (NLL: sort diagnostics by span) - #52905 (Fix a typo in unsize.rs) - #52907 (NLL: On "cannot move out of type" error, print original before rewrite) - #52908 (Use SetLenOnDrop in Vec::truncate()) - #52914 (Only run the sparc-abi test on sparc) - #52918 (Backport 1.27.2 release notes) - #52929 (Update compatibility note for 1.28.0 to be correct) Failed merges: - #52758 (Cleanup for librustc::session) - #52799 (Use BitVector for global sets of AttrId) r? @ghost
⌛ Testing commit ca762ba with merge 752e6a4b62628b2755cbc8c47fc1e6600ef3a085... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors: retry |
rustc: Disallow machine applicability in foreign macros Recent changes to lints disallowed lints from being emitted against code located in foreign macros, except for future-incompatible lints. For a future incompatible lint, however, the automatic suggestions may not be applicable! This commit updates this code path to force all applicability suggestions made to foreign macros to never be `MachineApplicable`. This should avoid rustfix actually attempting fixing these suggestions, causing non-compiling code to be produced. Closes rust-lang/cargo#5799
cc travis-ci/travis-ci#4924 for the travis hosts shutting down |
☀️ Test successful - status-appveyor, status-travis |
Recent changes to lints disallowed lints from being emitted against code located
in foreign macros, except for future-incompatible lints. For a future
incompatible lint, however, the automatic suggestions may not be applicable!
This commit updates this code path to force all applicability suggestions made
to foreign macros to never be
MachineApplicable
. This should avoid rustfixactually attempting fixing these suggestions, causing non-compiling code to be
produced.
Closes rust-lang/cargo#5799