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

[WIP] add check if lints belong to an external macro #49755

Conversation

Dylan-DPC-zz
Copy link

Closes #48855

r? @Manishearth

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2018
@TimNN

This comment has been minimized.

@TimNN

This comment has been minimized.

@Manishearth
Copy link
Member

Alright, this is a good first step -- now, the LintStore contains a hashmap of future incompatibility lints; we should always lint for these, regardless of the macro. So check the lint id against that.

@pietroalbini pietroalbini added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2018
@pietroalbini
Copy link
Member

Ping from triage @Dylan-DPC! The reviewer requested some changes, could you implement them in the near future?

@Dylan-DPC-zz
Copy link
Author

Yes sure. Will add them in a few days 👍

@Dylan-DPC-zz
Copy link
Author

@Manishearth I don't have access to the LintStore in the place where i'm checking the lint. Any clues?

@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2018
@pietroalbini
Copy link
Member

Ping from triage @Manishearth! The author needs a bit of help.

@Manishearth
Copy link
Member

Sure you do -- .lints()

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2018
@rust-highfive

This comment has been minimized.

@shepmaster
Copy link
Member

Ping from triage, @Dylan-DPC ! You have test failures to address.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:07:40]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:07:42] error[E0432]: unresolved import `codemap`
[00:07:42]   --> librustc/lint/mod.rs:57:5
[00:07:42]    |
[00:07:42] 57 | use codemap::{ExpnFormat, ExpnInfo, Span };
[00:07:42]    |     ^^^^^^^ Did you mean `syntax::codemap`?
[00:07:45] error: unused import: `Span`
[00:07:45]   --> librustc/lint/mod.rs:57:37
[00:07:45]    |
[00:07:45]    |
[00:07:45] 57 | use codemap::{ExpnFormat, ExpnInfo, Span };
[00:07:45]    |
[00:07:45]    = note: `-D unused-imports` implied by `-D warnings`
[00:07:45] 
[00:07:58] error[E0308]: mismatched types
[00:07:58] error[E0308]: mismatched types
[00:07:58]    --> librustc/lint/context.rs:472:51
[00:07:58]     |
[00:07:58] 472 |                 if !lint::in_external_macro(lint, span) {
[00:07:58]     |                                                   ^^^^ expected struct `syntax_pos::Span`, found type parameter
[00:07:58]     = note: expected type `syntax_pos::Span`
[00:07:58]                found type `S`
[00:07:58] 
[00:07:58] 
[00:07:58] error[E0277]: the trait bound `lint::Lint: lint::context::LintContext<'_>` is not satisfied
[00:07:58]     |
[00:07:58]     |
[00:07:58] 472 |                 if !lint::in_external_macro(lint, span) {
[00:07:58]     |                     ^^^^^^^^^^^^^^^^^^^^^^^ the trait `lint::context::LintContext<'_>` is not implemented for `lint::Lint`
[00:07:58]     |
[00:07:58] note: required by `lint::in_external_macro`
[00:07:58]    --> librustc/lint/mod.rs:625:1
[00:07:58]     |
[00:07:58] 625 | pub fn in_external_macro<'a, T: LintContext<'a>>(cx: &T, span: Span) -> bool {
[00:07:58] 
[00:07:58] 
-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-946eff7380f27f57.rlib --extern rustc_apfloat=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_apfloat-b7c66a9cab3ff5a6.rlib --extern proc_macro=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libproc_macro-8d928be2ff984c7f.so --extern log=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblog-4f0866e958f59455.rlib --extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-b789a86e1ab64d11.so -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/backtrace-sys-31a3817325787acc/out/.libs -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/miniz-sys-90c24e5cae9d047e/out` (exit code: 101)
[00:08:11] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:08:11] expected success, got: exit code: 101
[00:08:11] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1091:9
[00:08:11] travis_fold:end:stage0-rustc

[00:08:11] travis_time:end:stage0-rustc:start=1525535135554418716,finish=1525535323351964844,duration=187797546128
[

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 @TimNN. (Feature Requests)

@pietroalbini
Copy link
Member

Ping from triage @Dylan-DPC! You have other failures to address.

@pietroalbini
Copy link
Member

Thank you for this PR @Dylan-DPC! Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again!

@Dylan-DPC-zz
Copy link
Author

Oops sorry, got busy and demotivated to complete this. Sure no problems 😄

bors added a commit that referenced this pull request Jul 20, 2018
Squash all lints tied to foreign macros by default

This PR is a continuation of #49755 (thanks for the initial jump-start @Dylan-DPC!) and is targeted at solving #48855. This change updates the lint infrastructure to, by default, ignore all lints emitted for code that originates in a foreign macro. For example if `println!("...")` injects some idiomatic warnings these are all ignored by default. The rationale here is that for almost all lints there's no action that can be taken if the code originates from a foreign lint.

Closes #48855
Closes #52483
Closes #52479
@Dylan-DPC-zz Dylan-DPC-zz deleted the feature/macro-checks-to-lints branch December 3, 2018 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants