Uh oh!
There was an error while loading. Please reload this page.
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
nikomatsakis
commented
Nov 3, 2017
@oli-obk I wonder if we can make a test for that |
oli-obk
commented
Nov 3, 2017
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 |
nikomatsakis
commented
Nov 6, 2017
@bors r+ |
bors
commented
Nov 6, 2017
📌 Commit 2961937 has been approved by |
bors
commented
Nov 9, 2017
Use a `Set<T>` instead of a `Map<T, bool>` r? @nikomatsakis introduced in #44501
bors
commented
Nov 9, 2017
☀️ Test successful - status-appveyor, status-travis |
r? @nikomatsakis
introduced in #44501