Skip to content

Add short error message-format#44636

Merged
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:little-error-msg
Oct 25, 2017
Merged

Add short error message-format#44636
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:little-error-msg

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

Fixes #42653.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@eddyb

eddyb commented Sep 16, 2017

Copy link
Copy Markdown
Contributor

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned eddyb Sep 16, 2017
@GuillaumeGomez GuillaumeGomez changed the title Add short message-format Add short error message-format Sep 17, 2017
@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 18, 2017
@nrc

nrc commented Sep 19, 2017

Copy link
Copy Markdown
Member

Rather than adding a new emitter, could you have the human-readable emitter take an argument for detail level - either short or verbose? It seems that we'd save a lot of code dup (and a macro) that way.

Could you put this behind a feature gate?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Yes and yes. I was just thinking that we'd want to fully split both of the features but if not...

@GuillaumeGomez GuillaumeGomez force-pushed the little-error-msg branch 6 times, most recently from 8468a5f to a58becd Compare September 19, 2017 21:58
@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Ok, now just remain to put it behind a feature flag.

@kennytm

kennytm commented Sep 20, 2017

Copy link
Copy Markdown
Member

Is it possible to make each error only occupy one line, similar to clang -fno-caret-diagnostics?

Mock up:

$DIR/short-error-format.rs:16:9: error[E0308]: mismatched types
$DIR/short-error-format.rs:18:7: error[E0599]: no method named `salut` found for type `u32` in the current scope
error: aborting due to 2 previous errors 

@eddyb

eddyb commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

@kenny mismatched types is the worst error in the entire compiler, if you can't see the detail lines. I know it looks pretty but it has screwed over pretty much everyone who doesn't look at the console, including the official RLS extension for VS Code.
I only recently pulled off a hack to have the information available to kate in a manageable way.

@kennytm

kennytm commented Sep 20, 2017

Copy link
Copy Markdown
Member

@eddyb That sample is referring to the UI test in this PR https://github.com/GuillaumeGomez/rust/blob/a58becd60d7d7a965ed246cf8e290e2bd94043b9/src/test/ui/short-error-format.stderr:

error[E0308]: mismatched types
  --> $DIR/short-error-format.rs:16:9

error[E0599]: no method named `salut` found for type `u32` in the current scope
  --> $DIR/short-error-format.rs:18:7

error: aborting due to 2 previous errors

Improving E0308 could happen independent of this PR.

@eddyb

eddyb commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

Oh yeah, in that case, I agree, the example output in #44636 (comment) is a subset of the lines I have from my errfilt conversion, but I also have labels/notes (within some limitations), e.g.:

src/librustc_trans/mir/lvalue.rs:97:62: error[E0308]: mismatched types
src/librustc_trans/mir/lvalue.rs:97:62: expected struct `rustc::ty::layout::FullLayout`, found enum `rustc::mir::tcx::LvalueTy`
src/librustc_trans/mir/lvalue.rs:97:62: note: expected type `rustc::ty::layout::FullLayout<'_>`
src/librustc_trans/mir/lvalue.rs:97:62: found type `rustc::mir::tcx::LvalueTy<'_>`

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

So which output do you want? So I can do it once and for all. :)

@GuillaumeGomez GuillaumeGomez force-pushed the little-error-msg branch 3 times, most recently from d598684 to 30128d6 Compare September 21, 2017 17:47
@estebank

Copy link
Copy Markdown
Contributor

I feel if the idea is to use as little vertical space the output should be closer to:

$DIR/short-error-format.rs:16:9 - error[E0308]: mismatched types
$DIR/short-error-format.rs:18:7 - error[E0599]: no method named `salut` found for type `u32` in the current scope

error: aborting due to 2 previous errors

Beyond that, I wonder wether it'd be worth it to try to keep in the diagnostic machinery space for custom single line error messages that would work better for short output (and for text editors). May be I'll just go ahead and introduce it for while and see how much of a pain it becomes. If it is too much, we can rip it out. I do feel strongly against improving things for text editors in detriment of cli output.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

New output available!

@estebank estebank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicks, but I'm ok with the PR. Leaving it to @nrc / @eddyb to give final r+.

Comment thread src/librustc/session/config.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

human, json or short

Comment thread src/librustc/session/mod.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fits in single line?

Comment thread src/librustc/session/mod.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fits in single line?

Comment thread src/librustc/session/mod.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fits in single line?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

The branch has been created on rustfmt so I point to it instead.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

We now have the following error:

[00:00:36] Submodule 'src/tools/rustfmt' (https://github.com/rust-lang-nursery/rustfmt.git) unregistered for path 'src/tools/rustfmt'
[00:00:36] Submodule 'src/tools/rustfmt' (https://github.com/rust-lang-nursery/rustfmt.git) registered for path 'src/tools/rustfmt'
[00:00:37] error: Server does not allow request for unadvertised object fc2c85d423a34086243f019c15e2d9cc2afcbb8f
[00:00:37] Fetched in submodule path 'src/tools/rustfmt', but it did not contain fc2c85d423a34086243f019c15e2d9cc2afcbb8f. Direct fetching of that commit failed.

Anyone has an idea?

@topecongiro

Copy link
Copy Markdown
Contributor

@GuillaumeGomez Could you please use e35e27659dec7a70d1e0e45be2c5f3a02b2c742c? I think I have erased fc2c85d423a34086243f019c15e2d9cc2afcbb8f while I pushed the update :/ I am sorry.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Ok, let's hope it'll be the good one this time. :)

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

New error and it doesn't get any better:

[00:02:06] extracting /checkout/obj/build/cache/2017-08-29/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:02:06] error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`
[00:02:06]
[00:02:06] Caused by:
[00:02:06] patch for `rustfmt-nightly` in `https://github.com/rust-lang/crates.io-index` did not resolve to any crates
[00:02:06] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:02:06] Build completed unsuccessfully in 0:00:46

Anyone has an idea?

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2017
@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Fixed.

@michaelwoerister

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Oct 25, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit b46c014 has been approved by michaelwoerister

@kennytm kennytm 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 25, 2017
@bors

bors commented Oct 25, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit b46c014 with merge f9d2416...

bors added a commit that referenced this pull request Oct 25, 2017
…ister

Add short error message-format

Fixes #42653.
@bors

bors commented Oct 25, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing f9d2416 to master...

@bors bors merged commit b46c014 into rust-lang:master Oct 25, 2017
@GuillaumeGomez GuillaumeGomez deleted the little-error-msg branch October 26, 2017 08:30
@alexcrichton

Copy link
Copy Markdown
Member

@nrc @GuillaumeGomez this landed without a feature flag, was that intentional? I assume not given #44636 (comment)?

@nrc

nrc commented Nov 15, 2017

Copy link
Copy Markdown
Member

I suppose this should be behind -Zunstable-features given that comment. I filed #45995

Thanks for noticing @alexcrichton !

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Hum indeed, it should had have a feature flag. I'll add it as soon as possible.

@vitiral

vitiral commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

❤️ Thanks for fixing the bug I reported, looking forward to using this when it becomes available!

@vitiral

vitiral commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

On this comment my preference would be something like:

src/librustc_trans/mir/lvalue.rs:97:62: error[E0308]: mismatched types. Expected struct `rustc::ty::layout::FullLayout`, found enum `rustc::mir::tcx::LvalueTy`

It's all in one line but that's probably okay. That's just, like, my opinion though man.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 31, 2018
…r-format, r=oli-obk

Stabilize short error format

r? @oli-obk

Added in rust-lang#44636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.