Miri refactor: Final round #53779
Miri refactor: Final round #53779
Conversation
…alTy constructor private Also remove public OpTy constructors, but a pub(crate) constructor remains
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit b06a8db has been approved by |
bors
added
S-waiting-on-bors
|
@bors r+ |
|
📌 Commit 97d693a has been approved by |
|
@bors p=21 (rollup fairness) |
Miri refactor: Final round Tying up some loose ends that I noticed in the previous PRs -- and finally getting argument passing into a shape where @eddyb says it is "okay", which is a big improvement over the previous verdict that I cannot quote in public. ;) Also move a bunch of useful helpers to construct `Scalar` from miri to here. Cc @eddyb r? @oli-obk
|
☀️ Test successful - status-appveyor, status-travis |
|
This is a small 5% loss for perf |
|
Interesting, function calls do one query less, but they are more careful about argument passing, so maybe that costs more than the query saves. |
| Scalar::Bits { bits, .. } => bits == 0, | ||
| Scalar::Ptr(_) => false | ||
| } | ||
| } |
There was a problem hiding this comment.
Uh... when did this happen? I feel it wasn't present in my original review, but I may have missed it.
I don't like this function because for pointers, it cannot reliably tell you whether this is NULL or not. There might have been overflow. That's why I deliberately removed this function recently.
There was a problem hiding this comment.
Oh lol, never mind, I am looking at the wrong PR. We have too many PRs called "miri refactor".^^
Tying up some loose ends that I noticed in the previous PRs -- and finally getting argument passing into a shape where @eddyb says it is "okay", which is a big improvement over the previous verdict that I cannot quote in public. ;)
Also move a bunch of useful helpers to construct
Scalarfrom miri to here.Cc @eddyb
r? @oli-obk