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

Output previous stable error messaging when using stable build. #65893

Merged
merged 1 commit into from Oct 29, 2019

Conversation

jafern14
Copy link

@jafern14 jafern14 commented Oct 28, 2019

Fixes #65254

As I had mentioned previously there I have the logic running right now however I'm not getting the exact same syntax highlighting as there was originally for this error.

I'm currently getting the following:

error: expected expression, found statement (`let`)
 --> src/main.rs:2:14
  |
2 |     let x = (let y = 6);
  |              ^^^^^^^^^
  |
  = note: variable declaration using `let` is a statement

I'd like to get the following instead:

  |     let x = (let y = 6);
  |              ^^^

My current understanding is that the span being passed into lower_expr_let is coming from lowering.rs. I still don't know how the byte range is calculated for the erroneous syntax and need to look into it a bit more. In the meantime does anybody have any hints/tips regarding this??

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@Centril
Copy link
Contributor

Centril commented Oct 28, 2019

r? @Centril

@rust-highfive rust-highfive assigned Centril and unassigned cramertj Oct 28, 2019
@jafern14
Copy link
Author

I forgot to undo the text change to the nightly feature error output which resulted in the majority of the test failures. At the time I was experimenting with building the compiler and figuring out how it came together. I removed that text to try and see a change in the compiler output with the local crate I was compiling and running.

I just put that text back in there. I'd like to add in a test to ensure nightly vs stable error output. You think this is worth it at all @Centril ? It would make the promoting process to fully stable a little bit more involved. Not sure what your best practices are here.

@Centril
Copy link
Contributor

Centril commented Oct 28, 2019

I'm currently getting the following:

This is good; better actually! -- the old message only highlighted the let because it didn't perform proper parser recovery. :)

Not sure what your best practices are here.

@estebank Do we have any way of forcing a compiler to believe it's a stable compiler for testing purposes via some command line flag or something? Otherwise I don't think we can test the difference.

@Centril Centril added the F-let_chains `#![feature(let_chains)]` label Oct 28, 2019
@estebank
Copy link
Contributor

@Centril, sadly I don't think we do. You can change the target of your build to test locally, but I haven't seen any tests actually testing what things look in stable (although we do have stable/nightly output differences which we should test).

@Centril
Copy link
Contributor

Centril commented Oct 28, 2019

@estebank I guess there's not much we can do in this case then.

r=me rollup with @estebank's nit addressed and the commits squashed into one.

@jafern14 jafern14 force-pushed the let-expr-stable-error-messaging branch from 88eaf05 to f1aa8b2 Compare October 28, 2019 19:49
@jafern14
Copy link
Author

Fixed the nit and squashed the commits.

@Centril
Copy link
Contributor

Centril commented Oct 28, 2019

Thanks! @bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 28, 2019

📌 Commit f1aa8b2 has been approved by Centril

@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 Oct 28, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 28, 2019
…aging, r=Centril

Output previous stable  error messaging when using stable build.

Fixes rust-lang#65254

As I had mentioned previously there I have the logic running right now however I'm not getting the exact same syntax highlighting as there was originally for this error.

I'm currently getting the following:
```
error: expected expression, found statement (`let`)
 --> src/main.rs:2:14
  |
2 |     let x = (let y = 6);
  |              ^^^^^^^^^
  |
  = note: variable declaration using `let` is a statement
```

I'd like to get the following instead:

```
  |     let x = (let y = 6);
  |              ^^^
```

My current understanding is that the `span` being passed into `lower_expr_let` is coming from `lowering.rs`. I still don't know how the byte range is calculated for the erroneous syntax and need to look into it a bit more. In the meantime does anybody have any hints/tips regarding this??
bors added a commit that referenced this pull request Oct 28, 2019
Rollup of 9 pull requests

Successful merges:

 - #65563 (Add long error explanation for E0587)
 - #65640 (Use heuristics to recover parsing of missing `;`)
 - #65643 (Correct handling of type flags with `ConstValue::Placeholder`)
 - #65825 (rustc: use IndexVec<DefIndex, T> instead of Vec<T>.)
 - #65858 (suggest `const_in_array_repeat_expression` flag)
 - #65877 (doc: introduce `once` in `iter::chain` document)
 - #65887 (doc: mention `get(_mut)` in Vec)
 - #65891 (self-profiling: Record something more useful for crate metadata generation event.)
 - #65893 (Output previous stable  error messaging when using stable build.)

Failed merges:

r? @ghost
@bors bors merged commit f1aa8b2 into rust-lang:master Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-let_chains `#![feature(let_chains)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
6 participants