Ignore deprecation for items deprecated by the same attribute #35317
Conversation
|
|
||
| bug!("unexpected lint traversal order: expected `_post` for {:?} but called for {:?}", | ||
| top, item_id); | ||
| } |
There was a problem hiding this comment.
Not entirely sure, is this a good use of the lint hooks and DefIndex?
The need for a Vec is a bit sad - manually written visitors can get away with just regular stack recursion.
cc @Manishearth @llogiq @nikomatsakis
There was a problem hiding this comment.
Remember that all the lints have the cost of a single visitor; they are threaded together. This cannot be done with arbitrary visitors. The lint hooks exist basically for this use case.
That said, can't we pull instead of push here? We can use parent nodes and walk up the node tree. The case of a lint being triggered is relatively rare, so paying the cost of a tree walk there isn't too bad.
There was a problem hiding this comment.
You mean we could use get_parent instead of a stack, and store a single NodeId?
I think that would work - the problem solved by the stack here is that not all NodeIds are in the map.
There was a problem hiding this comment.
Why do we need to store anything?
Also, I thought that was fixed? Which nodes are missing from the map?
There was a problem hiding this comment.
@Manishearth: The cases I encountered where Paths: for example Foo and Bar in impl Foo for Bar or fn f<T: Foo>() {}, if I remember correctly.
There was a problem hiding this comment.
Oh, I see. Hmm.
In that case,do what eddy suggested -- store a single node id, and dynamically walk up the tree from there on error.
There was a problem hiding this comment.
on error.
Not always in post_item?
|
LGTM, modulo the two comments I've left. |
|
Yeah, walking up the tree isn't so bad if we can get away without allocation in the non-error case. |
TimNN
force-pushed
the
internal-deprecated
branch
from
5895b6e
to
65dafe0
Compare
|
@bors r+ |
|
📌 Commit 65dafe0 has been approved by |
Whenever a node whould be reported as deprecated: - check if the parent item is also deprecated - if it is and both were deprecated by the same attribute - skip the deprecation warning fixes rust-lang#35128 closes rust-lang#16490
TimNN
force-pushed
the
internal-deprecated
branch
from
65dafe0
to
627b1e8
Compare
|
@eddyb could you r+ again, I had to fix a tidy failure. |
|
@bors r+ |
|
📌 Commit 627b1e8 has been approved by |
Whenever a node would be reported as deprecated:
fixes #35128
closes #16490
r? @eddyb