Improve uninit/zeroed lint #66044
Improve uninit/zeroed lint #66044
Conversation
|
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/librustc_lint/builtin.rs
Outdated
| } | ||
| } else if let hir::ExprKind::MethodCall(ref path, _, ref args) = expr.kind { | ||
| // Find problematic calls to `MaybeUninit::assume_init`. | ||
| if path.ident.name == sym::assume_init { |
There was a problem hiding this comment.
This one seems odd to me. I am comparing just the name of the method -- but what if there is a trait or another type or so that also had this method? This looks like I am working pre-resolution, which seems fragile. Isn't there any way to get the full path to the method that will actually be called here?
I looked at clippy but couldn't find anything less fragile there either. Cc @oli-obk @Manishearth
There was a problem hiding this comment.
Can you use diagnostics items instead? #60966
There was a problem hiding this comment.
Hm, I have an Option<HirId>; I guess I could unwrap that and then call owner_def_id to get the DefId that is_diagnostic_item needs.
There was a problem hiding this comment.
Nope, that does not work. The lint just never fires. Looks like the HirId I get there is not pointing to the method being called -- that kind of makes sense if the information we have here is pre-method-resolution. (Probably it's the HirId of the method call itself.)
For Call, I can call
cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?to get the def_id of the callee. Isn't there something similar for MethodCall?
There was a problem hiding this comment.
Hm, maybe I need to construct a QPath::TypeRelative? I have a PathSegment, but I'd still also need a Ty.
There was a problem hiding this comment.
Ah, I found a way:
cx.tables.type_dependent_def_id(expr.hir_id)?Here, expr is the MethodCall itself.
| if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind { | ||
| if let hir::ExprKind::Path(ref qpath) = path_expr.kind { | ||
| let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?; | ||
| if cx.match_def_path(def_id, MU_ZEROED_PATH) { |
There was a problem hiding this comment.
Shouldn't this (and the other one below) also use diagnostics items?
There was a problem hiding this comment.
Yes, see #66075. This is partially pre-existing. I didn't feel like rewriting the entire lint at once.^^
There was a problem hiding this comment.
yea we should be moving away from match_def_path where we can. That's the reason diagnostic items were introduced
There was a problem hiding this comment.
@RalfJung Sure that's fair about the old code... but these are additions?
There was a problem hiding this comment.
But I guess this is as good as any a time to resolve the first bullet point from that issue. Not sure when I'll get to it, though.
|
r? @Centril r=me with or without changing all the additions to using diagnostics items (hopefully that will happen in a follow up if not done here -- remember however to cleanup |
Centril
added
S-waiting-on-author
|
@bors r+ having a tracking issue seems fine. It's a WIP in clippy, too. |
|
📌 Commit bb37d00 has been approved by |
bors
added
S-waiting-on-bors
Improve uninit/zeroed lint * Also warn when creating a raw pointer with a NULL vtable. * Also identify `MaybeUninit::uninit().assume_init()` and `MaybeUninit::zeroed().assume_init()` as dangerous.
Improve uninit/zeroed lint * Also warn when creating a raw pointer with a NULL vtable. * Also identify `MaybeUninit::uninit().assume_init()` and `MaybeUninit::zeroed().assume_init()` as dangerous.
Rollup of 12 pull requests Successful merges: - #65794 (gate rustc_on_unimplemented under rustc_attrs) - #65945 (Optimize long-linker-command-line test) - #66044 (Improve uninit/zeroed lint) - #66076 (HIR docs: mention how to resolve method paths) - #66084 (Do not require extra LLVM backends for `x.py test` to pass) - #66111 (improve from_raw_parts docs) - #66114 (Improve std::thread::Result documentation) - #66117 (Fixed PhantomData markers in Arc and Rc) - #66146 (Remove unused parameters in `__thread_local_inner`) - #66147 (Miri: Refactor to_scalar_ptr out of existence) - #66162 (Fix broken link in README) - #66171 (Update link on CONTRIBUTING.md) Failed merges: r? @ghost
MaybeUninit::uninit().assume_init()andMaybeUninit::zeroed().assume_init()as dangerous.