Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify error on casting larger integers to char #91939

Merged
merged 2 commits into from
Feb 6, 2022

Conversation

GKFX
Copy link
Contributor

@GKFX GKFX commented Dec 14, 2021

Closes #91836 with changes to E0604.md and a span_help.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 14, 2021
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 14, 2021
@rust-log-analyzer

This comment has been minimized.

@@ -337,6 +337,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
self.expr_ty
)
.span_label(self.span, "invalid cast")
.span_help(self.span, "try `char::from_u32` instead")
Copy link
Member

@camelid camelid Dec 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be suggested if self.expr_ty is u32. Pseudocode:

Suggested change
.span_help(self.span, "try `char::from_u32` instead")
let err = type_error_struct!(...).span_label(...);
if self.expr_ty == fcx.tcx.types.u32 {
err.span_help(self.span, "try `char::from_u32` instead");
}
err.emit();

@camelid
Copy link
Member

camelid commented Dec 14, 2021

Once you do that, you'll need to update test output with ./x.py test --bless src/test/ui.

@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-diagnostics Area: Messages for errors, warnings, and lints and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2021
@JohnCSimon
Copy link
Member

Ping from triage:
@GKFX can you post your status on this PR?

@GKFX
Copy link
Contributor Author

GKFX commented Jan 24, 2022

Apologies for the late reply. I have one comment on the feedback. If someone tries to cast (e.g.) u16 to char, I think they still should be using char::from_u32. The alternative would be to cast from u16 to u8 and onto char, which obviously fails for input greater than 255.
Of course, if someone is trying to cast a wider value like i64 to char, they need to be a bit thoughtful about their approach, as char::from_u32(0xF_0000_00061i64 as u32) incorrectly returns Some('a'), but in general I think the message should be to use from_u32 for any integer except i8.

                if self.expr_ty.is_numeric() {
                    err.span_help(self.span, if self.expr_ty == fcx.tcx.types.i8 {
                        "try casting from `u8` instead"
                    } else if self.expr_ty == fcx.tcx.types.u32 {
                        "try `char::from_u32` instead"
                    } else {
                        "try `char::from_u32` instead (via a `u32`)"
                    });
                }

@rust-log-analyzer

This comment has been minimized.

@GKFX
Copy link
Contributor Author

GKFX commented Feb 3, 2022

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 3, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Feb 5, 2022

Thanks @GKFX.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 5, 2022

📌 Commit badb81a has been approved by cjgillot

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 5, 2022
Clarify error on casting larger integers to char

Closes rust-lang#91836 with changes to E0604.md and a `span_help`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2022
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#91939 (Clarify error on casting larger integers to char)
 - rust-lang#92300 (mips64-openwrt-linux-musl: Add Tier 3 target)
 - rust-lang#92383 (Add new target armv7-unknown-linux-uclibceabi (softfloat))
 - rust-lang#92651 (Remove "up here" arrow on item-infos)
 - rust-lang#93556 (Change struct expr pretty printing to match rustfmt style)
 - rust-lang#93649 (Add regression tests for issue 80309)
 - rust-lang#93657 (Update CPU idle tracking for apple hosts)
 - rust-lang#93659 (Refactor conditional)
 - rust-lang#93669 (Resolve lifetimes for const generic defaults)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 58bfe72 into rust-lang:master Feb 6, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 6, 2022
@GKFX GKFX deleted the feature-91866 branch May 3, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"only u8 can be cast as char, not i32" needs further information
8 participants