Rollup of 7 pull requests #65854
Closed
Rollup of 7 pull requests #65854
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
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
This test was actually about the unreachable_code flag, not dead_code, so I renamed it for clarity (to prepare for the next commit, where I plan to move a bunch of the dead_code tests to a single folder)
This helps organize the tests better. I also renamed several of the tests to remove redundant dead-code in the path, and better match what they're testing
This is leftover from a restructuring of lint registration for drivers; it should now happen via the register_lints field on Config rather than this function.
This allows us to directly pass in a lint buffer
According to @estebank, def_span scans forward on the line until it finds a {, and if it can't find one, fallse back to the span for the whole item. This was apparently written before the identifier span was explicitly tracked on each node. This means that if an unused function signature spans multiple lines, the entire function (potentially hundreds of lines) gets flagged as dead code. This could, for example, cause IDEs to add error squiggly's to the whole function. By using the span from the ident instead, we narrow the scope of this in most cases. In a wider sense, it's probably safe to use ident.span instead of def_span in most locations throughout the whole code base, but since this is my first contribution, I kept it small. Some interesting points that came up while I was working on this: - I reorganized the tests a bit to bring some of the dead code ones all into the same location - A few tests were for things unrelated to dead code (like the path-lookahead for parens), so I added #![allow(dead_code)] and cleaned up the stderr file to reduce noise in the future - The same fix doesn't apply to const and static declarations. I tried adding these cases to the match expression, but that created a much wider change to tests and error messages, so I left it off until I could get some code review to validate the approach.
…omatsakis Point at associated type for some obligations Partially address rust-lang#57663.
…nikomatsakis rustc: add `Span`s to `inferred_outlives_of` predicates. This would simplify rust-lang#59789, and I suspect it has some potential in diagnostics (although we don't seem to use the predicate `Span`s much atm).
…low-fundamental-local, r=nikomatsakis Coherence should allow fundamental types to impl traits when they are local After rust-lang#64414, `impl<T> Remote for Box<T> { }` is disallowed, but it is also disallowed in liballoc, where `Box` is a local type! Enabling `#![feature(re_rebalance_coherence)]` in `liballoc` results in: ``` error[E0210]: type parameter `F` must be used as the type parameter for some local type (e.g., `MyStruct<F>`) --> src\liballoc\boxed.rs:1098:1 | 1098 | impl<F: ?Sized + Future + Unpin> Future for Box<F> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `F` must be used as the type parameter for some local type ``` This PR relaxes `uncover_fundamental_ty` to skip local fundamental types. I didn't add a test since `liballoc` already fails to compile, but I can add one if needed. r? @nikomatsakis cc rust-lang#63599
…nsion, r=davidtwco Don't ICE for completely unexpandable `impl Trait` types Save the resolution of these types (to themselves) to the typeck tables so that they will eventually reach E0720. closes rust-lang#65561
Use ident.span instead of def_span in dead-code pass Hello! First time contributor! :) This should fix rust-lang#58729. According to @estebank in the duplicate rust-lang#63064, def_span scans forward on the line until it finds a {, and if it can't find one, falls back to the span for the whole item. This was apparently written before the identifier span was explicitly tracked on each node. This means that if an unused function signature spans multiple lines, the entire function (potentially hundreds of lines) gets flagged as dead code. This could, for example, cause IDEs to add error squiggly's to the whole function. By using the span from the ident instead, we narrow the scope of this in most cases. In a wider sense, it's probably safe to use ident.span instead of def_span in most locations throughout the whole code base, but since this is my first contribution, I kept it small. Some interesting points that came up while I was working on this: - I reorganized the tests a bit to bring some of the dead code ones all into the same location - A few tests were for things unrelated to dead code (like the path-lookahead for parens), so I added #![allow(dead_code)] and cleaned up the stderr file to reduce noise in the future - The same fix doesn't apply to const and static declarations. I tried adding these cases to the match expression, but that created a much wider change to tests and error messages, so I left it off until I could get some code review to validate the approach.
…omatsakis Remove lint callback from driver This is leftover from a restructuring of lint registration for drivers; it should now happen via the register_lints field on Config rather than this function. This is not used by anyone to my knowledge (including the compiler itself); it was introduced in an abandoned refactor in rust-lang#65193.
…, r=nikomatsakis Remove LintBuffer from Session This moves the `LintBuffer` from `Session` into the `Resolver`, where it is used until lowering is done and then consumed by early lint passes. This also happily removes the failure mode of buffering lints too late where it would have previously lead to ICEs; it is statically no longer possible to do so. I suspect that with a bit more work a similar move could be done for the lint buffer inside `ParseSess`, but this PR doesn't touch it (in part to keep itself small). The last commit is the "interesting" commit -- the ones before it don't work (though they compile) as they sort of prepare the various crates for the lint buffer to be passed in rather than accessed through Session.
|
@bors r+ p=7 rollup=never |
|
📌 Commit ee82c8c has been approved by |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit ee82c8c with merge d9185c63e8ec818a9d8941abe087af5cda69ccd6... |
|
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 |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
This was referenced Oct 27, 2019
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.
Successful merges:
Spans toinferred_outlives_ofpredicates. #65541 (rustc: addSpans toinferred_outlives_ofpredicates.)impl Traittypes #65777 (Don't ICE for completely unexpandableimpl Traittypes)Failed merges:
r? @ghost