Use a Set<T> instead of a Map<T, bool>#45736
Conversation
| let body_id = tcx.hir.body_owned_by(node_id); | ||
| let body_hir_id = tcx.hir.node_to_hir_id(body_id.node_id); | ||
| tcx.rvalue_promotable_map(def_id).contains_key(&body_hir_id.local_id) | ||
| tcx.rvalue_promotable_map(def_id).contains(&body_hir_id.local_id) |
There was a problem hiding this comment.
I did this exact thing in clippy, and it was very wrong. I'm fairly certain it was wrong here, too.
This checked whether the local_id was ever checked for rvalue promotion. Not whether it is rvalue promotable.
| -> cmt<'tcx> { | ||
| let hir_id = self.tcx.hir.node_to_hir_id(id); | ||
| let promotable = self.rvalue_promotable_map.as_ref().map(|m| m[&hir_id.local_id]) | ||
| let promotable = self.rvalue_promotable_map.as_ref().map(|m| m.contains(&hir_id.local_id)) |
There was a problem hiding this comment.
This is the only change in semantics. We do not panic anymore if local_id was never checked for rvalue promotion, we just return false
|
@oli-obk I wonder if we can make a test for that |
|
I wondered the same thing, but found it very hard to actually do. There are too many other checks, it was probably redundant anyway, since it would additionally have to go through trans const eval |
|
@bors r+ |
|
📌 Commit 2961937 has been approved by |
Use a `Set<T>` instead of a `Map<T, bool>` r? @nikomatsakis introduced in #44501
|
☀️ Test successful - status-appveyor, status-travis |
r? @nikomatsakis
introduced in #44501