Fewer early errors

[nnethercote]

r? @davidtwco

[nnethercote]

@davidtwco: I think this is probably desirable. It does add two #[allow(rustc::untranslatable_diagnostic)] #[allow(rustc::diagnostic_outside_of_impl)] pairs which aren't good. I wasn't quite sure how to do the error handling in the face of the error! macros within parse_cfg and parse_cfg_check.

[nnethercote]

The job x86_64-gnu-llvm-16 failed

Ugh, another early warning was added since I last updated my repo. I've fixed the compile error, but it required another #[allow(rustc::untranslatable_diagnostic)] #[allow(rustc::diagnostic_outside_of_impl)] pair. I should fix it properly, but I don't have the energy today.

[davidtwco]

This is good, a few more untranslatable diagnostics aren't the worst in the world, we can fix those with time. r=me with CI passing unless you think the changes will need another look.

[nnethercote]

I fixed the test failures. The problem was that jobserver warnings are now done via Handler, which means they now contribute to the warning count, and that warning count now gets printed at the end of execution.

I'm quite uncertain about whether this PR should be merged, because it causes us to count this compiler invocation warnings the same as warnings about the code being compiled. The same thing now happens with self-profile warnings, e.g. we used to do this:

$ rustc +nightly -Zself-profile=/ a.rs
warning: failed to create profiler: Permission denied (os error 13)

and now we do this:

$ rustc +nightly -Zself-profile=/ a.rs
warning: failed to create profiler: Permission denied (os error 13)

warning: 1 warning emitted

Or, for a program with a code warning, we go from this:

$ rustc +nightly -Zself-profile=/ a.rs
warning: failed to create profiler: Permission denied (os error 13)

warning: unused variable: `x`
 --> a.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

to this:

$ rustc +nightly -Zself-profile=/ a.rs
warning: failed to create profiler: Permission denied (os error 13)


warning: unused variable: `x`
 --> a.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: 2 warnings emitted

[davidtwco]

I fixed the test failures. The problem was that jobserver warnings are now done via Handler, which means they now contribute to the warning count, and that warning count now gets printed at the end of execution.

I'm quite uncertain about whether this PR should be merged, because it causes us to count this compiler invocation warnings the same as warnings about the code being compiled. The same thing now happens with self-profile warnings, e.g. we used to do this:

I'm not sure that this is an issue. I think these are as much warnings as those about the code being compiled, they're there because the user would probably want to know what those warnings are telling them, and so should be included in the overall count. I'd maybe feel differently if these were warnings that, for some invocations, would just appear unconditionally, but I don't think that's the case here.

I think the early error functions mostly existed because we didn't always have a Handler available at this point in the past, not because we wanted these warnings to be excluded from the warning count.

[davidtwco]

@bors r+

[bors]

📌 Commit 6184099 has been approved by davidtwco

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6184099 with merge 7df0c21...

[bors]

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 7df0c21 to master...

[rust-timer]

Finished benchmarking commit (7df0c21): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.5%	[1.2%, 3.8%]	2
Regressions ❌ (secondary)	1.9%	[0.8%, 3.5%]	6
Improvements ✅ (primary)	-1.1%	[-2.5%, -0.4%]	11
Improvements ✅ (secondary)	-3.4%	[-5.5%, -2.2%]	5
All ❌✅ (primary)	-0.5%	[-2.5%, 3.8%]	13

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.4%, 0.5%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-0.5%, 0.5%]	3

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 674.699s -> 676.203s (0.22%)
Artifact size: 314.20 MiB -> 314.21 MiB (0.00%)