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

Scope prints in diagnostics #2777

Closed
wants to merge 1 commit into from

Conversation

da-x
Copy link
Member

@da-x da-x commented Oct 4, 2019

Summary

The proposed change is to add context information in warnings and errors, so that not only source positions are printed, but also item names. For example, in fn main, or in fn mod::function, or in struct Foo. Also, relevant lines in annotation are added (but not repetitively).

error[E0124]: field `x` is already declared
 --> test.rs:9:5  in struct Foo
  |
1 | struct Foo {
...
6 |     x: u64,
  |     ------ `x` first declared here
...
9 |     x: u64,
  |     ^^^^^^ field already declared

Instead of:

error[E0124]: field `x` is already declared
 --> test.rs:9:5
  |
6 |     x: u64,
  |     ------ `x` first declared here
...
9 |     x: u64,
  |     ^^^^^^ field already declared

Related links

@da-x da-x force-pushed the scope-prints-in-diagnostics branch from b8ce648 to a3b6de6 Compare October 4, 2019 21:13
@Centril Centril added A-diagnostics Proposals relating to diagnostics (error messages). T-compiler Relevant to the compiler subteam, which will review and decide on the RFC. labels Oct 4, 2019
@Centril Centril requested a review from estebank October 4, 2019 21:19
@estebank
Copy link
Contributor

estebank commented Oct 4, 2019

We can already do the following:

error[E0124]: field `x` is already declared
 --> test.rs:9:5  in struct Foo
  |
1 | struct Foo {
           ---
...
6 |     x: u64,
  |     ------ `x` first declared here
...
9 |     x: u64,
  |     ^^^^^^ field already declared

but it needs to be done on every error manually. Although this approach requires more effort, it also is more targeted, which means that we only show it when relevant. The devil's in the details, but the implementation of this seems "hard".

@da-x
Copy link
Member Author

da-x commented Oct 5, 2019

@estebank yeah, it's hard. There's also the issue of avoiding redundant scope prints when we have two or more different types of errors in the same scope [1], which probably means tracking scopes and perform unification of several unrelated errors based on their scope. Perhaps this can be done outside the call sites to emit diagnostics (i.e. only in librustc_errors), so that changes would not need to be tree-wide, making them easier. However, there's the issue of how to actually format the prints in that case. I wasn't able yet to come up with something sensible enough to put in the RFC as an alternative, but I'd like to mention it nonetheless.

On the bright side, simply avoiding duplicates in EmitterWriter based on Span, as I've done in the draft implementation is an easy way out implementation-wise, the result [2] may be good enough, perhaps.

[1]

error[E0124]: field `x` is already declared
 --> test.rs:9:5
  |
6 |     x: u64,
  |     ------ `x` first declared here
...
9 |     x: u64,
  |     ^^^^^^ field already declared

error[E0107]: wrong number of type arguments: expected 1, found 0
  --> test.rs:10:8
   |
10 |     z: Option<>,
   |        ^^^^^^^^ expected 1 type argument

[2]

error[E0124]: field `x` is already declared
 --> test.rs:9:5  in struct Foo
  |
1 | struct Foo {
...
6 |     x: u64,
  |     ------ `x` first declared here
...
9 |     x: u64,
  |     ^^^^^^ field already declared

error[E0107]: wrong number of type arguments: expected 1, found 0
  --> test.rs:10:8  in struct Foo
   |
10 |     z: Option<>,
   |        ^^^^^^^^ expected 1 type argument

@oli-obk
Copy link
Contributor

oli-obk commented Dec 14, 2019

cc @rust-lang/wg-diagnostics

@wesleywiser
Copy link
Member

I realize this RFC is quite old at this point but in case anyone is interested in this, I agree with @estebank's comment. If there are specific errors where this context would be helpful, I think we would be willing to just review PRs that target those specific errors. For more general or wide ranging changes, I think it would be best to open an MCP to discuss those changes.

@estebank
Copy link
Contributor

I'm going to close this RFC in favor of more targeted MCPs for specific error types.

@estebank estebank closed this Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Proposals relating to diagnostics (error messages). T-compiler Relevant to the compiler subteam, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants