don't lint on x = x + y inside an AddAssign impl #1318
Conversation
| // check that we are not inside an `impl AssignOp` of this exact operation | ||
| let parent_fn = cx.tcx.map.get_parent(e.id); | ||
| let parent_impl = cx.tcx.map.get_parent(parent_fn); | ||
| if parent_impl.as_u32() != 0 { |
There was a problem hiding this comment.
without this check, the get call in the next line will panic. All other functions on map will also panic, so there's no Option way to do this correctly. Should probably fix this in rustc...
There was a problem hiding this comment.
That's kind of a feature, Option<NodeId> is not necessary as there is syntax::ast::DUMMY_NODE_ID. Yup, that's weak typing, but it's more memory efficient, and NodeIds are used everywhere.
There was a problem hiding this comment.
uuuhh... once we get panic! inside const fn, we can properly implement NonZero::new and then we can go mad on the microoptimizations (most relevant here: Option<NonZero<NodeId>> being the same size as NodeId).
There was a problem hiding this comment.
I tried to compare parent_impl with DUMMY_NODE_ID, but DUMMY_NODE_ID is !0 and not 0 (see http://manishearth.github.io/rust-internals-docs/syntax/ast/constant.DUMMY_NODE_ID.html ).
There was a problem hiding this comment.
Ah, it's not a dummy id, it's the CRATE_NODE_ID, I pushed a fix. The reason for the check is that the crate root is not in the tcx.map, so tcx.map.get(CRATE_NODE_ID) will panic
| // check that we are not inside an `impl AssignOp` of this exact operation | ||
| let parent_fn = cx.tcx.map.get_parent(e.id); | ||
| let parent_impl = cx.tcx.map.get_parent(parent_fn); | ||
| if parent_impl.as_u32() != 0 { |
There was a problem hiding this comment.
That's kind of a feature, Option<NodeId> is not necessary as there is syntax::ast::DUMMY_NODE_ID. Yup, that's weak typing, but it's more memory efficient, and NodeIds are used everywhere.
|
rebased with just tiny changes to address rustc changes |
fixes #1302