"aborting due to N previous errors" only counts errors in the last pass

[arielb1]

Meta

$ rustc --version
rustc 1.9.0-beta.1 (37a2869af 2016-04-12)

STR

fn main() {
    a;
    "".lorem;
    "".ipsum;
}

Expected Results

3 errors are reported, followed by "error: aborting due to 3 previous errors".

Actual Result

3 errors are reported, but the errors reported count only counts the 2 errors from the type-checking pass - so "error: aborting due to 2 previous errors" is printed.

[nrc]

Nominating just to signal boost really since this is really annoying. I think it should be easy to fix, but I'm not quite confident enough about that to slap an e-easy on this.

[nikomatsakis]

triage: P-medium

@nrc would love to mentor a fix. @nrc, if you can write up a series of steps to fix at some point that'd be great!

[nrc]

@Mark-Simulacrum were you interested in working on this?

[Mark-Simulacrum]

Sure! I'd be willing to try getting this done. Can you give me a hint where I should look at?

[nrc]

@Mark-Simulacrum

So, this is pretty much a "hunt the bug" issue. I'm not exactly sure where it is going wrong, but once you find it is should be easy to fix.

Start by looking at run_compiler in src/librustc_driver/lib.rs, this returns a CompileResult which if there is an error, contains the number of errors found. I'm 99% sure the problem is not actually in that function but in the tail call to driver::compile_input, that function executes each phase of the compiler. I think what is going on is that one phase has m errors, then we continue compilation (we didn't used to do this) and then a later stage has n errors and then we stop compilation returning n when we should return n + m. Once you've got a test case, it should be pretty easy to find where we're doing this. The slightly tricky bit is making sure we cover all possible cases for getting the wrong number of errors. Note that we can bail out of compilation from deep inside the phases. So it might be that at the top level, we track the number of errors, but deep in some phase we are giving up with only the error count from that phase or sub-phase.

[Mark-Simulacrum]

Okay, updating this issue with what I've found and putting it up for grabs, since I think this is more I-papercut and as such P-low, but I'll leave that designation up to the compiler team.

Currently, name resolution, for example, does not track the errors that it reports, and we then continue despite it's failure, forgetting to update the number of errors.

After talking with @nrc on IRC, it's been suggested that a semi-temporary fix for this could be to change the "error: aborting due to 3 previous errors" text to "error: aborting due to a few previous errors" for 2-4 errors, "error: aborting due to many previous errors" for 5+ errors, etc.

I personally feel like the error count isn't altogether that useful as it is, except potentially as a way of assessing the time required to remove an obsolete function or other wide-sweeping change--though I feel like rustc potentially not reporting all errors sort of makes even that not really possible; so perhaps the "right" fix is to remove the error reporting from the output, though that's technically backwards incompatible, I guess.

[brson]

I think the only value this message has is giving a concrete number, but it would be worth while to question how valuable that actually is. If we can't get the number right, then the message is not useful - it's obvious from the spew there were multiple errors of some quantity. I'd be happy removing it completely. We have a lot of error spew, and this is any easy bit to scale back.

[Mark-Simulacrum]

I agree. I think that we can just change it to say "aborting due to previous errors" without giving a number, since that's arguably enough information. I don't really know of any cases where it'd be super helpful to know the quantity of errors a change introduced (and rustc | rg error | wc -l is a decent heuristic).

Just some quick steps if anyone wants to implement this (I can mentor). The following two places in the code need to be changed to print the "aborting due to previous error(s)" statement.

src/librustc_driver/lib.rs
123:        e => format!("aborting due to {} previous errors", e),

src/librustc_errors/lib.rs
506:                s = format!("aborting due to {} previous errors", self.err_count.get());

[citizen428]

@Mark-Simulacrum I'll pick this one up.

[Mark-Simulacrum]

This was fixed in #42150.