Support creating float constants in rustc_public mir#159472
Conversation
rustbot
added
S-waiting-on-review
|
r? @oli-obk rustbot has assigned @oli-obk. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| }) | ||
| } | ||
|
|
||
| /// Create a new constant that represents the given value. |
There was a problem hiding this comment.
Do you mind improving this doc a bit? I think we should state that the value is the binary representation of the float constant.
There was a problem hiding this comment.
I tried to improve the docs.
| let mut tables = self.tables.borrow_mut(); | ||
| let cx = &*self.cx.borrow(); | ||
| let ty = cx.new_rigid_ty(RigidTy::Float(float_ty).internal(&mut *tables, cx.tcx)); | ||
| cx.try_new_const_uint(value, ty).map(|cnst| cnst.stable(&mut *tables, cx)) |
There was a problem hiding this comment.
I think it might be worth creating a bridge for creating a float constant. This function eventually creates a Scalar::Integer. In practice it may not matter, but it's fragile.
There was a problem hiding this comment.
I had such a function in my original patch, then I found out that it is identical to try_new_const_uint. I added some comment to explain uint usage is not a mistake here, but I can add a distinct function in the bridge or rename the existing one if it is desired.
There was a problem hiding this comment.
Sorry, I read this wrong. I think this is fine as is. Thanks
|
r? @celinval |
6872df7 to
c4bda51
Compare
IIUC there is no test for the sibling For now, I'm focused on upstreaming what looks uncontroversial and symmetric to the rest of the codes, and internalize the rest of my patch, so that my code becomes buildable on nightly rustc. I can add a simple test, but I think a real test for this function and its siblings needs a generative API and we can defer the test if/when rustc_public gains generative capabilities (without generative capabilities all of these functions can be removed or become private). |
There was a problem hiding this comment.
Interesting, I'm curious to hear more about it. Feel free to start a thread in our zulip channel #project-rustc-public or create an issue on the project repo.
I'd still prefer seeing this tested even in a follow up PR. So we can merge this as is, but can you please add tests in your next PR? I understand that similar functions currently don't have any test, but I don't want to keep making the situation worse..
Thanks
@bors r+ rollup
rust-bors
Bot
added
S-waiting-on-bors
Rollup of 14 pull requests Successful merges: - #159307 (Improve cross-namespace name diagnostics) - #159543 (Remove extra semicolons in parsing item lists) - #157270 (ergonomic_clones_dotuse_capture_by_ref: Capture upvar by ref for `.use` in non-move closures) - #158496 (Move `check_rustc_pub_transparent` into the attribute parser) - #158547 (Move `std::io::buffered` to `alloc::io`) - #158808 (Filter host libstdc++ ABI flag in rustc_llvm cross builds) - #159362 (Add regression test for #120328) - #159472 (Support creating float constants in rustc_public mir) - #159505 (make rustdoc::bare_urls strip trailing periods from url) - #159568 (Suggest close compiler options) - #159578 (Extract coroutine closure helper functions) - #159601 (Make `TokenTreeCursor` private) - #159613 (Set the rustc lib path for unstable-book-gen) - #159616 (Clarify the comment about stage1/stage2 discrepancy in input-stats test)
Rollup merge of #159472 - hkalbasi:rustc_public_float, r=celinval Support creating float constants in rustc_public mir This PR adds a `try_from_float` similar to `try_from_uint` to the `rustc_public::ty::MirConst`. It takes `u128` and users need a `.to_bits() as u128` to call this function. I did this because `f64` couldn't capture a `f128` const completely, and `f128` needs to cast and a branch on the input types, making the code complex. Another alternative is adding 4 functions `try_from_f16` to `try_from_f128`, which I don't like, but that's also an option.
Rollup of 14 pull requests Successful merges: - rust-lang/rust#159307 (Improve cross-namespace name diagnostics) - rust-lang/rust#159543 (Remove extra semicolons in parsing item lists) - rust-lang/rust#157270 (ergonomic_clones_dotuse_capture_by_ref: Capture upvar by ref for `.use` in non-move closures) - rust-lang/rust#158496 (Move `check_rustc_pub_transparent` into the attribute parser) - rust-lang/rust#158547 (Move `std::io::buffered` to `alloc::io`) - rust-lang/rust#158808 (Filter host libstdc++ ABI flag in rustc_llvm cross builds) - rust-lang/rust#159362 (Add regression test for rust-lang/rust#120328) - rust-lang/rust#159472 (Support creating float constants in rustc_public mir) - rust-lang/rust#159505 (make rustdoc::bare_urls strip trailing periods from url) - rust-lang/rust#159568 (Suggest close compiler options) - rust-lang/rust#159578 (Extract coroutine closure helper functions) - rust-lang/rust#159601 (Make `TokenTreeCursor` private) - rust-lang/rust#159613 (Set the rustc lib path for unstable-book-gen) - rust-lang/rust#159616 (Clarify the comment about stage1/stage2 discrepancy in input-stats test)
4 participants
This PR adds a
try_from_floatsimilar totry_from_uintto therustc_public::ty::MirConst.It takes
u128and users need a.to_bits() as u128to call this function. I did this becausef64couldn't capture af128const completely, andf128needs to cast and a branch on the input types, making the code complex. Another alternative is adding 4 functionstry_from_f16totry_from_f128, which I don't like, but that's also an option.