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

Rollup of 8 pull requests #121415

Merged
merged 29 commits into from
Feb 22, 2024
Merged

Rollup of 8 pull requests #121415

merged 29 commits into from
Feb 22, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

davidtwco and others added 29 commits February 20, 2024 10:25
Signed-off-by: David Wood <david@davidtw.co>
Compiletest code sometimes refers to a test's revision as its `cfg`. This
results in two different names for the same thing, one of which is ambiguous
with other kinds of configuration (such as compiletest's own config).

This patch replaces those occurrences of `cfg` with `revision`.
As bonus this makes the errors when failing to load a proc macro more
informative to match the backend loading errors. In addition it makes it
slightly easier to patch rustc to work on platforms that don't support
dynamic linking like wasm.
Its one use isn't necessary, because it's not possible for errors to
have been emitted at that point.
Currently `has_errors` excludes lint errors. This commit changes it to
include lint errors.

The motivation for this is that for most places it doesn't matter
whether lint errors are included or not. But there are multiple places
where they must be includes, and only one place where they must not be
included. So it makes sense for `has_errors` to do the thing that fits
the most situations, and the new `has_errors_excluding_lint_errors`
method in the one exceptional place.

The same change is made for `err_count`. Annoyingly, this requires the
introduction of `err_count_excluding_lint_errs` for one place, to
preserve existing error printing behaviour. But I still think the change
is worthwhile overall.
Currently `emit_stashed_diagnostic` is called from four(!) different
places: `print_error_count`, `DiagCtxtInner::drop`, `abort_if_errors`,
and `compile_status`.

And `flush_delayed` is called from two different places:
`DiagCtxtInner::drop` and `Queries`.

This is pretty gross! Each one should really be called from a single
place, but there's a bunch of entanglements. This commit cleans up this
mess.

Specifically, it:
- Removes all the existing calls to `emit_stashed_diagnostic`, and adds
  a single new call in `finish_diagnostics`.
- Removes the early `flush_delayed` call in `codegen_and_build_linker`,
  replacing it with a simple early return if delayed bugs are present.
- Changes `DiagCtxtInner::drop` and `DiagCtxtInner::flush_delayed` so
  they both assert that the stashed diagnostics are empty (i.e.
  processed beforehand).
- Changes `interface::run_compiler` so that any errors emitted during
  `finish_diagnostics` (i.e. late-emitted stashed diagnostics) are
  counted and cannot be overlooked. This requires adding
  `ErrorGuaranteed` return values to several functions.
- Removes the `stashed_err_count` call in `analysis`. This is possible
  now that we don't have to worry about calling `flush_delayed` early
  from `codegen_and_build_linker` when stashed diagnostics are pending.
- Changes the `span_bug` case in `handle_tuple_field_pattern_match` to a
  `delayed_span_bug`, because it now can be reached due to the removal
  of the `stashed_err_count` call in `analysis`.
- Slightly changes the expected output of three tests. If no errors are
  emitted but there are delayed bugs, the error count is no longer
  printed. This is because delayed bugs are now always printed after the
  error count is printed (or not printed, if the error count is zero).

There is a lot going on in this commit. It's hard to break into smaller
pieces because the existing code is very tangled. It took me a long time
and a lot of effort to understand how the different pieces interact, and
I think the new code is a lot simpler and easier to understand.
Because it's now simple enough that it doesn't provide much benefit.
It currently is infallible and uses `abort_if_errors` and
`FatalError.raise()` to signal errors. It's easy to instead return a
`Result<_, ErrorGuaranteed>`, which is the more usual way of doing
things.
Replace `abort_if_errors` calls that are certain to abort -- because
we emit an error immediately beforehand -- with `FatalErro.raise()`.
It's clumsy and doesn't improve readability.
…ng, r=oli-obk

Top level error handling

The interactions between the following things are surprisingly complicated:
- `emit_stashed_diagnostics`,
- `flush_delayed`,
- normal return vs `abort_if_errors`/`FatalError.raise()` unwinding in the call to the closure in `interface::run_compiler`.

This PR disentangles it all.

r? `@oli-obk`
coverage: Remove `pending_dups` from the span refiner

When extracting coverage spans from a function's MIR, we need to decide how to handle spans that are associated with more than one node (BCB) in the coverage control flow graph.

The existing code for managing those duplicate spans is very subtle and difficult to modify. But by eagerly deduplicating those extracted spans in a much simpler way, we can remove a massive chunk of complexity from the span refiner.

There is a tradeoff here, in that we no longer try to retain *all* nondominating BCBs that have the same span, only the last one in the (semi-arbitrary) dominance ordering. But in practice, this produces very little difference in our coverage tests, and the simplification is so significant that I think it's worthwhile.

``@rustbot`` label +A-code-coverage
… r=wesleywiser

triagebot: add queue notifications

This seems like it could be useful.

r? ``@wesleywiser``
Consistently refer to a test's `revision` instead of `cfg`

Compiletest allows a test file to specify multiple “revisions” (`//@ revisions: foo bar`), with each revision running as a separate test, and having the ability to define revision-specific headers (`//`@[foo]` ignore-blah`) and revision-specific code (`#[cfg(foo)]`).

The code that implements this feature sometimes uses the term “cfg” instead of “revision”. This results in two confusingly-different names for the same concept, one of which is ambiguous with other kinds of configuration (such as compiletest's own config).

This PR replaces those occurrences of `cfg` with `revision`, so that one name is used consistently.
…-errors

never patterns: Fix liveness analysis in the presence of never patterns

There's a bunch of code that only looks at the first alternative of an or-pattern, under the assumption that all alternatives have the same set of bindings. This is true except for never pattern alternatives  (e.g. `Ok(x) | Err(!)`), so we skip these. I expect there's other code with this problem, I'll have to check that later.

I don't have tests for this yet because mir lowering causes other issues; I'll have some in the next PR.

r? ``@compiler-errors``
…ochenkov

Unify dylib loading between proc macros and codegen backends

As bonus this makes the errors when failing to load a proc macro more informative to match the backend loading errors. In addition it makes it slightly easier to patch rustc to work on platforms that don't support dynamic linking like wasm.
…trochenkov

Solaris linker does not support --strip-debug

Fixes rust-lang#121381
@rustbot rustbot added the A-meta Area: Issues about the rust-lang/rust repository. label Feb 21, 2024
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Feb 21, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Feb 21, 2024

📌 Commit 35650a4 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors 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 Feb 21, 2024
@bors
Copy link
Contributor

bors commented Feb 22, 2024

⌛ Testing commit 35650a4 with merge d8b0069...

@bors
Copy link
Contributor

bors commented Feb 22, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing d8b0069 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 22, 2024
@bors bors merged commit d8b0069 into rust-lang:master Feb 22, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 22, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#121206 Top level error handling 5088f1454658b811d6e7156341c693bceb2c05fc (link)
#121261 coverage: Remove pending_dups from the span refiner d6463dc041fe64ae2df17b3e3eae41970fa6d7a5 (link)
#121336 triagebot: add queue notifications 269873b9b704f9130d7bfa75ee4e8eea617c6399 (link)
#121373 Consistently refer to a test's revision instead of cfg 602a93422bc5e5f97c89095ad57f2b4c82069234 (link)
#121391 never patterns: Fix liveness analysis in the presence of ne… d454b6fb15055b07ac42598fdfcf60eaf40c02ac (link)
#121392 Unify dylib loading between proc macros and codegen backends ea0e660d405068e3f9df08ca6de38da077be42ed (link)
#121399 Solaris linker does not support --strip-debug 6900c3c09ef2d68e034a8fab817de84ddc132e4c (link)
#121406 Add a couple tests 6e460072de95e945f5168c135d93c432b749dfa7 (link)

previous master: 3406ada96f

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d8b0069): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)
- - 0
Regressions ❌
(secondary)
3.2% [2.2%, 3.9%] 3
Improvements ✅
(primary)
-3.3% [-4.4%, -2.3%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -3.3% [-4.4%, -2.3%] 2

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
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 651.719s -> 649.976s (-0.27%)
Artifact size: 310.97 MiB -> 310.93 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues about the rust-lang/rust repository. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet