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

When seeing x<-y in a boolean expression, suggest x < -y. #45651

Closed
kennytm opened this issue Oct 31, 2017 · 6 comments
Closed

When seeing x<-y in a boolean expression, suggest x < -y. #45651

kennytm opened this issue Oct 31, 2017 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-frontend Area: Compiler frontend (errors, parsing and HIR) C-feature-request Category: A feature request, i.e: not implemented / a PR. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics

Comments

@kennytm
Copy link
Member

kennytm commented Oct 31, 2017

Test case:

fn main() {
    let x = -5;
    if x<-1 {
        println!("ok");
    }
}

The error currently looks like:

error: placement-in expression syntax is experimental and subject to change. (see issue #27779)
 --> src/main.rs:3:8
  |
3 |     if x<-1 {
  |        ^^^^
  |
  = help: add #![feature(placement_in_syntax)] to the crate attributes to enable

but the intent was not to use the placement-in, the suggestion is just confusing the user. If we do add #![feature(placement_in_syntax)] the error message would be even worse:

error[E0277]: the trait bound `{integer}: std::ops::Placer<_>` is not satisfied
 --> src/main.rs:5:8
  |
5 |     if x<-1 {
  |        ----
  |        |
  |        the trait `std::ops::Placer<_>` is not implemented for `{integer}`
  |        in this macro invocation
  |
  = help: the following implementations were found:
            <std::collections::linked_list::BackPlace<'a, T> as std::ops::Placer<T>>
            <std::collections::vec_deque::PlaceBack<'a, T> as std::ops::Placer<T>>
            <std::collections::hash_map::Entry<'a, K, V> as std::ops::Placer<V>>
            <&'a mut std::collections::BinaryHeap<T> as std::ops::Placer<T>>
          and 4 others
  = note: required by `std::ops::Placer::make_place`

The suggestion should actually mention inserting a space between < and - if a comparison with a negative number is intended, e.g.

error: placement-in expression syntax is experimental and subject to change. (see issue #27779)
 --> src/main.rs:3:8
  |
3 |     if x<-1 {
  |        ^^^^
  |        hint: for less-than comparison, try to add some spaces: `x < -1`
  |
  = help: add #![feature(placement_in_syntax)] to the crate attributes to enable
@kennytm kennytm added A-diagnostics Area: Messages for errors, warnings, and lints A-frontend Area: Compiler frontend (errors, parsing and HIR) C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Oct 31, 2017
@estebank estebank added E-needs-mentor WG-diagnostics Working group: Diagnostics labels Oct 31, 2017
@estebank
Copy link
Contributor

estebank commented Dec 13, 2017

Should expand the following code:

ast::ExprKind::InPlace(..) => {
gate_feature_post!(&self, placement_in_syntax, e.span, EXPLAIN_PLACEMENT_IN);

to have a version gate_feature_post! that accepts extra notes, and add a note that if you meant to compare x with 1 you should add a space between < and - (you can get the spans from the two fields in InPlace(expr, expr).

The same should be done for the case where the feature flag has been enabled:

error[E0277]: the trait bound `{integer}: std::ops::Placer<_>` is not satisfied
 --> src/main.rs:4:8
  |
4 |     if x<-1 {
  |        ----
  |        |
  |        the trait `std::ops::Placer<_>` is not implemented for `{integer}`
  |        in this macro invocation
  |
  = help: the following implementations were found:
            <std::vec::PlaceBack<'a, T> as std::ops::Placer<T>>
            <std::collections::linked_list::FrontPlace<'a, T> as std::ops::Placer<T>>
            <std::collections::vec_deque::PlaceFront<'a, T> as std::ops::Placer<T>>
            <std::collections::vec_deque::PlaceBack<'a, T> as std::ops::Placer<T>>
          and 4 others
  = note: required by `std::ops::Placer::make_place`

I believe it is somewhere under:

ExprKind::InPlace(ref placer, ref value_expr) => {

The second case can also be dealt with by using rustc_on_unimplemented to add a special note/label for all integer types.

@estebank estebank added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. and removed E-needs-mentor labels Dec 13, 2017
@arshiamufti
Copy link
Contributor

Can I take this on? I have some time in the latter half of this month.

@estebank
Copy link
Contributor

estebank commented Jan 9, 2018

@arshiamufti feel free to do so and to ask for help!

@PramodBisht
Copy link
Contributor

@estebank Hi, Can I take this one? if yes, then I will need your guidance.

@arshiamufti
Copy link
Contributor

hi @PramodBisht -- i'm actually going to work on this sometime in the near future :)

@PramodBisht
Copy link
Contributor

@arshiamufti sure!

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 30, 2018
…rkor

Suggest correct comparison against negative literal

When parsing as emplacement syntax (`x<-1`), suggest the correct syntax
for comparison against a negative value (`x< -1`).

Fix rust-lang#45651.
kennytm added a commit to kennytm/rust that referenced this issue Jun 30, 2018
…rkor

Suggest correct comparison against negative literal

When parsing as emplacement syntax (`x<-1`), suggest the correct syntax
for comparison against a negative value (`x< -1`).

Fix rust-lang#45651.
bors added a commit that referenced this issue Jul 1, 2018
Suggest correct comparison against negative literal

When parsing as emplacement syntax (`x<-1`), suggest the correct syntax
for comparison against a negative value (`x< -1`).

Fix #45651.
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 A-frontend Area: Compiler frontend (errors, parsing and HIR) C-feature-request Category: A feature request, i.e: not implemented / a PR. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

4 participants