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

Beta regression in span printing with tabs involved #47377

Closed
est31 opened this issue Jan 12, 2018 · 6 comments · Fixed by #47407
Closed

Beta regression in span printing with tabs involved #47377

est31 opened this issue Jan 12, 2018 · 6 comments · Fixed by #47407
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@est31
Copy link
Member

est31 commented Jan 12, 2018

This code (containing tabs) gives an error (playground):

	let b = "hello";
	let _a = b + ", World!";

The error works fine on stable:

  |
3 |  let _a = b + ", World!";
  |           ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
3 |  let _a = b.to_owned() + ", World!";
  |           ^^^^^^^^^^^^

But is mispositioned on rustc 1.25.0-nightly (73ac5d6 2018-01-11) as well as on rustc 1.24.0-beta.2 (a19122c 2018-01-10):

  |
3 |     let _a = b + ", World!";
  |              ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
3 |     let _a = b.to_owned() + ", World!";
  |           ^^^^^^^^^^^^

cc @estebank

@BubbaSheen
Copy link

BubbaSheen commented Jan 12, 2018 via email

@est31
Copy link
Member Author

est31 commented Jan 12, 2018

Using bisect-rust I could see that the issue started with #45953, but probably the actual issue is somewhere else (e.g. in the machinery added in #45711 that #45953 starts using for tabs).

@est31
Copy link
Member Author

est31 commented Jan 12, 2018

Yeah, the issue also exists when emojis are involved (no tabs):

    let b = "hello";
    println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";

Gives:

4 |     println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
  |                                 ^^^^^^^^^^^^

The emoji manifestation of the issue is no beta regression, seems to exist on stable as well.

@est31
Copy link
Member Author

est31 commented Jan 12, 2018

I've created #47380 for the emoji/unicode issue.

@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 12, 2018
@estebank
Copy link
Contributor

estebank commented Jan 12, 2018

It seems to me that the issue only manifests itself in suggestions, and that is handled separately from the main diagnostics.


Mentoring instructions:

emit_suggestion_default needs to be modified to use col_display instead of col.0, the start being span_start_pos.col_display + start and the end being span_start_pos.col_display + start + sub_len (sub_len already accounts for unicode chars). This change should fix both this and #47380.

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jan 12, 2018
@gaurikholkar-zz
Copy link

Will pick this up

kennytm added a commit to kennytm/rust that referenced this issue Jan 17, 2018
fix mispositioned span

This fixes rust-lang#47377

The output now looks like this
```
error[E0369]: binary operation `+` cannot be applied to type `&str`
 --> h.rs:3:11
  |
3 |     let _a = b + ", World!";
  |              ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
3 |     let _a = b.to_owned() + ", World!";
  |              ^^^^^^^^^

error: aborting due to previous error
```
For the case when emojis are involved,  it gives the new output for proper indentation.
But for an indentation as follows,
```
fn main() {
let b = "hello";
    let _a = b + ", World!";
}
```
it still mispositions the span
```
3 |     println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
  |                                           ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
  |
3 |     println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
  |                                           ^^^^^^^
error: aborting due to previous erro
```

cc @estebank  @est31
kennytm added a commit to kennytm/rust that referenced this issue Jan 17, 2018
fix mispositioned span

This fixes rust-lang#47377

The output now looks like this
```
error[E0369]: binary operation `+` cannot be applied to type `&str`
 --> h.rs:3:11
  |
3 |     let _a = b + ", World!";
  |              ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
3 |     let _a = b.to_owned() + ", World!";
  |              ^^^^^^^^^

error: aborting due to previous error
```
For the case when emojis are involved,  it gives the new output for proper indentation.
But for an indentation as follows,
```
fn main() {
let b = "hello";
    let _a = b + ", World!";
}
```
it still mispositions the span
```
3 |     println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
  |                                           ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
  |
3 |     println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
  |                                           ^^^^^^^
error: aborting due to previous erro
```

cc @estebank  @est31
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 E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants