-
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
annotate stricter lifetimes on LateLintPass methods to allow them to forward to a Visitor #38191
Conversation
…forward to a Visitor
r? @eddyb Not sure of this. Sort of makes sense. |
@@ -131,51 +131,104 @@ pub trait LintPass { | |||
// contains a few lint-specific methods with no equivalent in `Visitor`. | |||
pub trait LateLintPass: LintPass { |
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.
Could you add the lifetimes to the trait? That way they don't pollute the method generics.
I'm not sure in what conditions they don't need to show up in types in the method signatures in an impl
- see how some of the Visitor
impl
s get away with it, although here it's trickier - it'd be nice if most of them didn't need to be changed.
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.
I might just be failing at HKL, but I can't create a Box<for<'a, 'tcx: 'a> LateLintPass<'a, 'tcx>>
, because the outlives bound is ignored
I have a workaround, but it'll require an intermediate helper trait. (see minimal example of workaround: https://is.gd/OnC7RS)
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.
Bleah I forgot about trait objects. Can the Helper
trait have a single blanket impl proxying to the trait with the lifetime paramters?
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.
I can almost do it (https://is.gd/TrZudu), I'm not sure why Rust can't figure out that a lifetime on a generic bound in the trait generics will outlive the lifetime of a method argument:
impl<'b, 'tcx: 'b, T: Foo<'b, 'tcx>> Helper<'tcx> for T {
fn as_foo<'a>(&'a mut self) -> &'a mut Foo<'a, 'tcx> where 'tcx: 'a {
self
}
}
note: the lifetime 'a as defined on the block at 17:72...
note: ...does not necessarily outlive the lifetime 'b as defined on the block at 17:72
it works now, I tested it against clippy, |
fn check_generics(&mut self, cx: &LateContext, it: &hir::Generics) { | ||
fn check_generics(&mut self, | ||
cx: &LateContext<'a, 'tcx>, | ||
it: &'tcx hir::Generics) { |
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.
Indentation looks off here.
I think some of the indentation changes from adding |
9f3757c
to
b650bbe
Compare
b650bbe
to
0f7a18b
Compare
cleaned up all the |
This also blocks Servo. |
Would appreciate a quick review here :) |
@nikomatsakis this was what I was just talking about |
@bors r+ p=1 (unbreaking Servo) |
📌 Commit 0f7a18b has been approved by |
annotate stricter lifetimes on LateLintPass methods to allow them to forward to a Visitor this unblocks clippy (rustup blocked after #37918) clippy has lots of lints that internally call an `intravisit::Visitor`, but the current lifetimes on `LateLintPass` methods conflicted with the required lifetimes (there was no connection between the HIR elements and the `TyCtxt`) r? @Manishearth
this unblocks clippy (rustup blocked after #37918)
clippy has lots of lints that internally call an
intravisit::Visitor
, but the current lifetimes onLateLintPass
methods conflicted with the required lifetimes (there was no connection between the HIR elements and theTyCtxt
)r? @Manishearth