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

Stop prepending the path and line number to every line in error messages #3533

Closed
bstrie opened this issue Sep 19, 2012 · 21 comments
Closed
Assignees
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.

Comments

@bstrie
Copy link
Contributor

bstrie commented Sep 19, 2012

For the sake of people with 80-character terminals, Rust code is limited to 78 characters in width. So why do error messages on 80-character terminals look like this:

/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:24:0: 29:1 wa
rning: type, variant, or trait must be camel case
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:24 trait file
search {
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:25     fn sys
root() -> Path;
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:26     fn lib
_search_paths() -> ~[Path];
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:27     fn get
_target_lib_path() -> Path;
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:28     fn get
_target_lib_file_path(file: &Path) -> Path;
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs:29 }

This is a pathological case, but I still manage to hit this frequently even though my term is often well wider than 80 characters.

Here's how Java formats its error messages:

jello.java:3: cannot find symbol
symbol  : variable Exception
location: class jello
        throw Exception;
              ^
1 error

Taking inspiration from this, Rust error messages could instead look like:

Warning: type, variant, or trait must be camel case
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs 24:0-29:1
trait filesearch {
    fn sysroot() -> Path;
    fn lib_search_paths() -> ~[Path];
    fn get_target_lib_path() -> Path;
    fn get_target_lib_file_path(file: &Path) -> Path;
}
@brson
Copy link
Contributor

brson commented Sep 19, 2012

We could also print the relative path instead of absolute. Whatever the solution it needs to be compatible with emacs's compilation buffers.

@nikomatsakis
Copy link
Contributor

Brian Anderson wrote:

We could also print the relative path instead of absolute. Whatever
the solution in needs to be compatible with emacs's compilation buffers.

I wanted to say the same thing. The current situation is nice (for me)
because emacs never has trouble resolving the filename (not always the
case for other compilers!)

@bstrie
Copy link
Contributor Author

bstrie commented Sep 20, 2012

Does compatibility with emacs' compilation buffers require prepending paths and line numbers to each line?

@brson
Copy link
Contributor

brson commented Sep 20, 2012

Emacs supports a variety of error reporting formats, so I imagine we have some flexibility. Finding out exactly what it accepts may require reading the code.

We will need to make sure it's ok with vim users to.

@brson
Copy link
Contributor

brson commented Sep 20, 2012

I think the answer is no though. If emacs decides a line is an error message it will try to interpret it, otherwise it will just print it. Some lines must be crafted to look special to emacs, other lines must be careful to not look special.

@Dretch
Copy link
Contributor

Dretch commented Sep 20, 2012

Building on @bstrie's example, I think it would be even better if it was made explicit which type, variant of trait is being referred to, like:

Warning: type, variant, or trait must be camel case
/home/ben/code/rust-projects/rust/src/rustc/metadata/filesearch.rs 24:7-24:17
23:
24: trait filesearch {
          ^
25:    fn sysroot() -> Path;
26:    fn lib_search_paths() -> ~[Path];

(I also changed the span in question to just cover the trait name, rather than the whole trait block).

@bstrie
Copy link
Contributor Author

bstrie commented Sep 20, 2012

@Dretch I believe that's been noted in #3522.

@catamorphism
Copy link
Contributor

Bumping to 0.7

@catamorphism
Copy link
Contributor

Not critical for 0.7, de-milestoning.

@emberian
Copy link
Member

Nominating for production ready.

B-Clarifying to work out the exact output desired.

@pnkfelix
Copy link
Member

not accepted for production ready

@huonw
Copy link
Member

huonw commented Aug 27, 2013

Traige: still a problem. My preference is for relative paths in the error/warnings/notes and no paths/spans on the code itself. Some examples of other compilers (all invoked <foo> 3533.<ext>):

Haskell

main = putStrLn 1

GHC

[1 of 1] Compiling Main             ( 3533.hs, 3533.o )

3533.hs:1:17:
    No instance for (Num String) arising from the literal `1'
    Possible fix: add an instance declaration for (Num String)
    In the first argument of `putStrLn', namely `1'
    In the expression: putStrLn 1
    In an equation for `main': main = putStrLn 1

Works with M-x compile.

Hugs

runhugs: Error occurred
ERROR "3533.hs":1 - Unresolved top-level overloading
*** Binding             : main
*** Outstanding context : Num [Char]

Doesn't work.

C

int main() { main + main; }

GCC

3533.c: In function ‘main’:
3533.c:1:19: error: invalid operands to binary + (have ‘int (*)()’ and ‘int (*)()’)
 int main() { main + main; }
                   ^

Works.

Clang

3533.c:1:19: error: invalid operands to binary expression ('int (*)()' and 'int (*)()')
int main() { main + main; }
             ~~~~ ^ ~~~~
1 error generated.

Works.

D

module main;
void main() { return 1; }

LDC2

3533.d(2): Error: long has no effect in expression (1)
3533.d(2): Error: cannot return non-void from void function

Doesn't work.

GDC

3533.d:2: Error: long has no effect in expression (1)
3533.d:2: Error: cannot return non-void from void function

Mostly works (the Error: seems to get interpreted as the column, but the jump-to-error functionality still works).

Go

package main
func main() { "foo" + 1 }

play.golang.org

prog.go:2: cannot convert "foo" to type int
prog.go:2: invalid operation: "foo" + 1 (mismatched types string and int)

(My package manager isn't letting me install straight golang atm.)

GCCGO

3533.go:2:21: error: value computed is not used
 func main() { "foo" + 1 }
                     ^

Works.

@pnkfelix
Copy link
Member

Does compatibility with emacs' compilation buffers require prepending paths and line numbers to each line?

The answer to this question was indeed "no." (I've become much more intimate with compilation mode since tackling #6887.)

@flaper87
Copy link
Contributor

Triage: Still valid. The path is now relative but it's still printed for every line. I also prefer file and line numbers in errors/warnings/notes messages and nothing on the code

trait test {}
fn main() {}
test-test/test.rs:1:1: 1:14 warning: trait `test` should have a camel case name such as `Test`, #[warn(non_camel_case_types)] on by default
test-test/test.rs:1 trait test {}

@reem
Copy link
Contributor

reem commented Dec 15, 2014

It's also annoying for readers that errors usually print very long lines that wrap and are very hard to read rather than splitting them up intelligently so that they look good even on trim terminals.

@tbelaire
Copy link
Contributor

tbelaire commented Jul 2, 2015

No. #11715 just fixes the line numbers to line up properly when we go from 9 to 10, or 99 to 100, as the formatting broke by one space. If we removed the line numbers and column numbers, it would be moot.

@Manishearth Manishearth added the E-help-wanted Call for participation: Help is requested to fix this issue. label Nov 20, 2015
@Manishearth
Copy link
Member

Relevant code:

for &line_number in display_lines.iter() {

We should split that to handle one-line and multi-line errors, keeping one-line errors as is and indenting multi-line errors by a fixed amount instead of appending the line number.

@Manishearth
Copy link
Member

Should there be a CLI flag for turning this off?

@Manishearth
Copy link
Member

@wafflespeanut is working on this

Note: We should preserve the old behavior with a rust flag like --machine-readable-errors or something for IDEs

@bstrie
Copy link
Contributor Author

bstrie commented Nov 21, 2015

I'm fine with having a flag to re-enable verbose error output for the purpose of IDEs, but the other comments in this thread seem to imply that's not actually necessary. What IDEs need output like this?

@Boddlnagg
Copy link
Contributor

VisualRust is parsing rustc output, but that's actually not very robust at the moment, so we need to change something there anyway. I'm also looking forward to having a structured machine-readable output format as proposed in https://github.com/nrc/rfcs/blob/ide/text/0000-ide.md#error-format. I'd say that the way forward is to have separate output formats for humans (focusing on readability) and IDEs, but the latter should not be what we have right now.

@Manishearth Manishearth removed the E-help-wanted Call for participation: Help is requested to fix this issue. label Feb 29, 2016
bors added a commit that referenced this issue Apr 30, 2016
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: #33240

Joint work with @jonathandturner.

Fixes #3533
bors added a commit that referenced this issue Apr 30, 2016
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: #33240

Joint work with @jonathandturner.

Fixes #3533
Manishearth added a commit to Manishearth/rust that referenced this issue May 2, 2016
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: rust-lang#33240

Joint work with @jonathandturner.

Fixes rust-lang#3533
bors added a commit that referenced this issue May 3, 2016
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: #33240

Joint work with @jonathandturner.

Fixes #3533
Manishearth added a commit to Manishearth/rust that referenced this issue May 3, 2016
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: rust-lang#33240

Joint work with @jonathandturner.

Fixes rust-lang#3533
bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
fix Comma in comment causes no formatting
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
…eferences, r=RalfJung

Make file descriptors into refcount references

fixes rust-lang#3525

Remove `fn dup` in `trait FileDescription`, define `struct FileDescriptor(Rc<RefCell<dyn FileDescription>>)`, and use `BTreeMap<i32, FileDescriptor>` in `FdTable`.

---

There are some refactors similar to the following form:
```rust
{  // origin:
    if let Some(file_descriptor) = this.machine.fds.get_mut(fd) {
        // write file_descriptor
        this.try_unwrap_io_result(result)
    } else {
        this.fd_not_found()
    }
}
{  // now:
    let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else {
        return this.fd_not_found();
    };
    // write file_descriptor
    drop(file_descriptor);
    this.try_unwrap_io_result(result)
}
```
The origin form can't compile because as using `RefCell` to get interior mutability, `fn get_mut` return `Option<std::cell::RefMut<'_, dyn FileDescription>>` instead of `Option<&mut dyn FileDescription>` now, and the `deref_mut` on `file_descriptor: RefMut` will cause borrow `this` as mutable more than once at a time.
So this form of refactors and manual drops are are implemented to avoid borrowing `this` at the same time.
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
…eferences, r=RalfJung

Make file descriptors into refcount references

fixes rust-lang#3525

Remove `fn dup` in `trait FileDescription`, define `struct FileDescriptor(Rc<RefCell<dyn FileDescription>>)`, and use `BTreeMap<i32, FileDescriptor>` in `FdTable`.

---

There are some refactors similar to the following form:
```rust
{  // origin:
    if let Some(file_descriptor) = this.machine.fds.get_mut(fd) {
        // write file_descriptor
        this.try_unwrap_io_result(result)
    } else {
        this.fd_not_found()
    }
}
{  // now:
    let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else {
        return this.fd_not_found();
    };
    // write file_descriptor
    drop(file_descriptor);
    this.try_unwrap_io_result(result)
}
```
The origin form can't compile because as using `RefCell` to get interior mutability, `fn get_mut` return `Option<std::cell::RefMut<'_, dyn FileDescription>>` instead of `Option<&mut dyn FileDescription>` now, and the `deref_mut` on `file_descriptor: RefMut` will cause borrow `this` as mutable more than once at a time.
So this form of refactors and manual drops are are implemented to avoid borrowing `this` at the same time.
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

15 participants