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

Logical right and left shift diagnostics. #102067

Closed
wants to merge 4 commits into from

Conversation

moonheart08
Copy link
Contributor

This adds lints for <<< and >>>. They explicitly do not suggest replacements as I could not figure out a simple way to do so for >>>, and for <<< the replacement is subjective (it could be <<, but that doesn't preserve sign.)

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 20, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022
@moonheart08 moonheart08 changed the title Arl arr diag Arithmetic right and left shift diagnostics. Sep 20, 2022
Comment on lines 50 to 51
.arithmetic_left_shift_operator_invalid = `<<<` is not a valid left shift operator, consider shifting normally and fixing the sign as needed.
.arithmetic_right_shift_operator_invalid = `>>>` is not a valid right shift operator, consider casting to a signed integer and right shifting normally.
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't end sentences with period (nor we capitalize the first letter in sentences)

Suggested change
.arithmetic_left_shift_operator_invalid = `<<<` is not a valid left shift operator, consider shifting normally and fixing the sign as needed.
.arithmetic_right_shift_operator_invalid = `>>>` is not a valid right shift operator, consider casting to a signed integer and right shifting normally.
.arithmetic_left_shift_operator_invalid = `<<<` is not a valid left shift operator, consider shifting normally and fixing the sign as needed
.arithmetic_right_shift_operator_invalid = `>>>` is not a valid right shift operator, consider casting to an unsigned integer and right shifting normally

I would prefer if the consider... text was instead a note, separate from the main label

Copy link
Member

Choose a reason for hiding this comment

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

I agree with both of these comments.

compiler/rustc_parse/src/parser/expr.rs Show resolved Hide resolved
Comment on lines 50 to 51
.arithmetic_left_shift_operator_invalid = `<<<` is not a valid left shift operator, consider shifting normally and fixing the sign as needed.
.arithmetic_right_shift_operator_invalid = `>>>` is not a valid right shift operator, consider casting to a signed integer and right shifting normally.
Copy link
Member

Choose a reason for hiding this comment

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

I agree with both of these comments.

@davidtwco
Copy link
Member

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned davidtwco Sep 21, 2022
@bors
Copy link
Contributor

bors commented Sep 21, 2022

☔ The latest upstream changes (presumably #101846) made this pull request unmergeable. Please resolve the merge conflicts.

@moonheart08 moonheart08 changed the title Arithmetic right and left shift diagnostics. Logical right and left shift diagnostics. Sep 21, 2022
@Nilstrieb
Copy link
Member

Hi, I've seen you changed some diagnostic structs in your PR. After #103345, the way we refer to fluent messages changed. They are now in a flat namespace with the same identifier as in the fluent file. For example, parser::cool_thing is now parser_cool_thing and parser::suggestion just suggestion.
You should rebase to the latest master and change your fluent message references as described above. Thanks!

@moonheart08
Copy link
Contributor Author

Ah shoot, this slipped under my radar. I'll put finishing this on my calender.

@apiraino
Copy link
Contributor

apiraino commented Nov 9, 2022

Switching to waiting on author to incorporate changes. Feel free to request a review with @rustbot ready, thanks!

@rustbot author

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

@moonheart08 any updates on this?

@Dylan-DPC
Copy link
Member

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks

@Dylan-DPC Dylan-DPC added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 16, 2023
@Dylan-DPC

This comment was marked as spam.

@Dylan-DPC Dylan-DPC closed this May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. 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.

None yet

10 participants