Internal lints take 2 #59316
Merged
Internal lints take 2 #59316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
flip1995
commented
Mar 20, 2019
oli-obk
reviewed
Mar 20, 2019
oli-obk
reviewed
Mar 20, 2019
oli-obk
reviewed
Mar 20, 2019
|
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 |
|
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 |
flip1995
force-pushed
the
internal_lints_take_2
branch
from
dd43979
to
16264e2
Compare
|
impl lgtm now |
|
It seems like the If I try to activate the lint group, I get an unknown lint error: |
|
That's very curious. There's probably a hack that makes if stage != "0" {
cmd.arg("-Zunstable-options");
}to rust/src/bootstrap/bin/rustc.rs Line 285 in 86466a3 |
Centril
added
S-waiting-on-author
|
Ping from triage, @flip1995 ^ |
|
Will get to this today / Monday. |
flip1995
force-pushed
the
internal_lints_take_2
branch
from
16264e2
to
c592b86
Compare
|
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 |
flip1995
force-pushed
the
internal_lints_take_2
branch
from
c592b86
to
223f7c5
Compare
flip1995
commented
Mar 31, 2019
flip1995
commented
Mar 31, 2019
oli-obk
reviewed
Apr 2, 2019
flip1995
force-pushed
the
internal_lints_take_2
branch
from
196c89b
to
ebde526
Compare
|
☔ The latest upstream changes (presumably #59657) made this pull request unmergeable. Please resolve the merge conflicts. |
oli-obk
requested changes
Apr 3, 2019
I thought about adding |
|
For |
flip1995
force-pushed
the
internal_lints_take_2
branch
from
3edc722
to
5422111
Compare
bors
added
the
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 3, 2019
…li-obk Internal lints take 2 cc rust-lang#58701 cc rust-lang#49509 TODO: Add `#![warn(internal)]` to crates (and fix violations) Crates depending on `rustc_data_structures` - [x] librustc_resolve - [x] librustc_driver - [x] librustc_passes - [x] librustc_metadata - [x] librustc_interface - [x] librustc_save_analysis - [x] librustc_lint - [x] librustc - [x] librustc_incremental - [x] librustc_codegen_utils - [x] libarena - [x] librustc_target - [x] librustc_allocator - [x] librustc_privacy - [x] librustc_traits - [x] librustc_borrowck - [x] libsyntax - [x] librustc_codegen_ssa - [x] libsyntax_ext - [x] librustc_errors - [x] librustc_mir - [x] libsyntax_pos - [x] librustc_typeck Crates with `feature(rustc_private)` Excluding crates, which are already in the list above. Also excluding tools and tests. - [ ] ~~libstd~~ - [x] libfmt_macros - [x] librustdoc r? @oli-obk
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 3, 2019
…li-obk Internal lints take 2 cc rust-lang#58701 cc rust-lang#49509 TODO: Add `#![warn(internal)]` to crates (and fix violations) Crates depending on `rustc_data_structures` - [x] librustc_resolve - [x] librustc_driver - [x] librustc_passes - [x] librustc_metadata - [x] librustc_interface - [x] librustc_save_analysis - [x] librustc_lint - [x] librustc - [x] librustc_incremental - [x] librustc_codegen_utils - [x] libarena - [x] librustc_target - [x] librustc_allocator - [x] librustc_privacy - [x] librustc_traits - [x] librustc_borrowck - [x] libsyntax - [x] librustc_codegen_ssa - [x] libsyntax_ext - [x] librustc_errors - [x] librustc_mir - [x] libsyntax_pos - [x] librustc_typeck Crates with `feature(rustc_private)` Excluding crates, which are already in the list above. Also excluding tools and tests. - [ ] ~~libstd~~ - [x] libfmt_macros - [x] librustdoc r? @oli-obk
bors
added a commit
that referenced
this pull request
Apr 4, 2019
Rollup of 6 pull requests Successful merges: - #59316 (Internal lints take 2) - #59663 (Be more direct about borrow contract) - #59664 (Updated the documentation of spin_loop and spin_loop_hint) - #59666 (Updated the environment description in rustc.) - #59669 (Reduce repetition in librustc(_lint) wrt. impl LintPass by using macros) - #59677 (rustfix coverage: Skip UI tests with non-json error-format) Failed merges: r? @ghost
eddyb
reviewed
Apr 4, 2019
|
|
||
| /// Check if a `DefId`'s path matches the given absolute type path usage. | ||
| // Uplifted from rust-lang/rust-clippy | ||
| pub fn match_path<'a, 'tcx>(self, tcx: TyCtxt<'a, 'tcx, 'tcx>, path: &[&str]) -> bool { |
There was a problem hiding this comment.
I'm sorry for not noticing this until now, but such functionality doesn't belong on DefId, not even TyCtxt IMO.
Could we move this to the lint context, instead?
There was a problem hiding this comment.
Yes, I will do this tomorrow/Saturday!
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 6, 2019
…anishearth Move match_path from DefId to lint::LateContext cc rust-lang#59316 (comment) r? @eddyb
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 6, 2019
…anishearth Move match_path from DefId to lint::LateContext cc rust-lang#59316 (comment) r? @eddyb
flip1995
added a commit
to flip1995/rust
that referenced
this pull request
Apr 7, 2019
This uplifts `get_def_path` from Clippy. This is a follow up on the implementation of internal lints: rust-lang#59316 The internal lint implementation also copied the implementation of the `AbsolutePathPrinter`. To get rid of this code duplication this also uplifts the `get_def_path` function from Clippy. This also renames `match_path` to `match_def_path`, as it was originally named in Clippy.
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 8, 2019
…ishearth Uplift `get_def_path` from Clippy cc rust-lang/rust-clippy#3926 cc rust-lang#59738 This uplifts `get_def_path` from Clippy. This is a follow up on the implementation of internal lints: rust-lang#59316 The internal lint implementation also copied the implementation of the `AbsolutePathPrinter`. To get rid of this code duplication this also uplifts the `get_def_path` function from Clippy. This also renames `match_path` to `match_def_path`, as it was originally named in Clippy. r? @Manishearth
sanxiyn
added a commit
to sanxiyn/rust
that referenced
this pull request
Apr 11, 2019
…ishearth Uplift `get_def_path` from Clippy cc rust-lang/rust-clippy#3926 cc rust-lang#59738 This uplifts `get_def_path` from Clippy. This is a follow up on the implementation of internal lints: rust-lang#59316 The internal lint implementation also copied the implementation of the `AbsolutePathPrinter`. To get rid of this code duplication this also uplifts the `get_def_path` function from Clippy. This also renames `match_path` to `match_def_path`, as it was originally named in Clippy. r? @Manishearth
flip1995
added a commit
to flip1995/rust
that referenced
this pull request
Apr 12, 2019
This uplifts `get_def_path` from Clippy. This is a follow up on the implementation of internal lints: rust-lang#59316 The internal lint implementation also copied the implementation of the `AbsolutePathPrinter`. To get rid of this code duplication this also uplifts the `get_def_path` function from Clippy. This also renames `match_path` to `match_def_path`, as it was originally named in Clippy.
mati865
reviewed
Apr 15, 2019
| @@ -109,6 +108,9 @@ pub fn create_session( | |||
| let codegen_backend = get_codegen_backend(&sess); | |||
|
|
|||
| rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess)); | |||
| if sess.unstable_options() { | |||
There was a problem hiding this comment.
Why was it guarded by unstable_options()?
There was a problem hiding this comment.
Because every lint, that is registered will be run, which is bad performance wise (for normal users) and can be easily prevented. We decided to not introduce a new -Z option and just use the unstable_options flag. #58701 (comment) #59316 (comment)
There was a problem hiding this comment.
These lints are useless outside of the compiler or clippy. Exposing them to stable would mean we'd need to support the name (or the lint existing) forever, even if the lint might disappear if the compiler datastructures change beyond the point where the lint is needed
There was a problem hiding this comment.
Oh, GitHub collapsed that comment.
I was wondering if there is better solution for Clippy and possibly other projects.
There was a problem hiding this comment.
Why not gating on whether the crate is compiled to be unstable with the feature rustc_private?
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 15, 2019
…ishearth Uplift `get_def_path` from Clippy cc rust-lang/rust-clippy#3926 cc rust-lang#59738 This uplifts `get_def_path` from Clippy. This is a follow up on the implementation of internal lints: rust-lang#59316 The internal lint implementation also copied the implementation of the `AbsolutePathPrinter`. To get rid of this code duplication this also uplifts the `get_def_path` function from Clippy. This also renames `match_path` to `match_def_path`, as it was originally named in Clippy. r? @Manishearth
3 tasks
Centril
added a commit
to Centril/rust
that referenced
this pull request
Jul 5, 2019
Implement another internal lints cc rust-lang#49509 This adds ~~two~~ one internal lint~~s~~: 1. LINT_PASS_IMPL_WITHOUT_MACRO: Make sure, that the `{declare,impl}_lint_pass` macro is used to implement lint passes. cc rust-lang#59669 2. ~~USAGE_OF_TYCTXT_AND_SPAN_ARGS: item 2 on the list in rust-lang#49509~~ ~~With 2. I wasn't sure, if this lint should be applied everywhere. That means a careful review of 0955835 would be great. Also 73fb9b4 allows this lint on some functions. Should I also apply this lint there?~~ TODO (not directly relevant for review): - [ ] rust-lang#59316 (comment) (not sure yet, if this works or how to query for `rustc_private`, since it's not in [`Features`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/feature_gate/struct.Features.html) 🤔 cc @eddyb) - [x] rust-lang#61735 (comment) - [x] Check explicitly for the `{declare,impl}_lint_pass!` macros r? @oli-obk
Centril
added a commit
to Centril/rust
that referenced
this pull request
Jul 5, 2019
Implement another internal lints cc rust-lang#49509 This adds ~~two~~ one internal lint~~s~~: 1. LINT_PASS_IMPL_WITHOUT_MACRO: Make sure, that the `{declare,impl}_lint_pass` macro is used to implement lint passes. cc rust-lang#59669 2. ~~USAGE_OF_TYCTXT_AND_SPAN_ARGS: item 2 on the list in rust-lang#49509~~ ~~With 2. I wasn't sure, if this lint should be applied everywhere. That means a careful review of 0955835 would be great. Also 73fb9b4 allows this lint on some functions. Should I also apply this lint there?~~ TODO (not directly relevant for review): - [ ] rust-lang#59316 (comment) (not sure yet, if this works or how to query for `rustc_private`, since it's not in [`Features`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/feature_gate/struct.Features.html) 🤔 cc @eddyb) - [x] rust-lang#61735 (comment) - [x] Check explicitly for the `{declare,impl}_lint_pass!` macros r? @oli-obk
Centril
added a commit
to Centril/rust
that referenced
this pull request
Jul 5, 2019
Implement another internal lints cc rust-lang#49509 This adds ~~two~~ one internal lint~~s~~: 1. LINT_PASS_IMPL_WITHOUT_MACRO: Make sure, that the `{declare,impl}_lint_pass` macro is used to implement lint passes. cc rust-lang#59669 2. ~~USAGE_OF_TYCTXT_AND_SPAN_ARGS: item 2 on the list in rust-lang#49509~~ ~~With 2. I wasn't sure, if this lint should be applied everywhere. That means a careful review of 0955835 would be great. Also 73fb9b4 allows this lint on some functions. Should I also apply this lint there?~~ TODO (not directly relevant for review): - [ ] rust-lang#59316 (comment) (not sure yet, if this works or how to query for `rustc_private`, since it's not in [`Features`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/feature_gate/struct.Features.html) 🤔 cc @eddyb) - [x] rust-lang#61735 (comment) - [x] Check explicitly for the `{declare,impl}_lint_pass!` macros r? @oli-obk
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc #58701
cc #49509
TODO: Add
#![warn(internal)]to crates (and fix violations)Crates depending on
rustc_data_structuresCrates with
feature(rustc_private)Excluding crates, which are already in the list above. Also excluding tools and tests.
libstdr? @oli-obk