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

rustc_span cleanups #117507

Merged
merged 9 commits into from
Nov 3, 2023
Merged

rustc_span cleanups #117507

merged 9 commits into from
Nov 3, 2023

Conversation

nnethercote
Copy link
Contributor

Just some things I found while looking over this crate.

r? @oli-obk

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Nov 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Nov 2, 2023

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

The comment just below the first one describes how the `impl !Send for
FatalError` makes it impossible to `panic!(FatalError)`.

And the second one should be `panic_any` instead of `panic!`.
And remove dead functions revealed by this.
@nnethercote
Copy link
Contributor Author

I have updated to address the review comments so far.

r? @Nilstrieb

@rustbot rustbot assigned Noratrieb and unassigned oli-obk Nov 2, 2023
@rust-log-analyzer

This comment has been minimized.

Most notably, this commit changes the `pub use crate::*;` in that file
to `use crate::*;`. This requires a lot of `use` items in other crates
to be adjusted, because everything defined within `rustc_span::*` was
also available via `rustc_span::source_map::*`, which is bizarre.

The commit also removes `SourceMap::span_to_relative_line_string`, which
is unused.
These are all called very rarely, so there is no need for them to be
inline.
With `create_default_session_globals_then`, which is preferable when it
is appropriate.
@@ -14,7 +14,7 @@ fn def_path_hash_depends_on_crate_id() {
// the crate by changing the crate disambiguator (e.g. via bumping the
// crate's version number).

create_session_if_not_set_then(Edition::Edition2024, |_| {
create_session_globals_then(Edition::Edition2024, || {
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't this panic if a second test was added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if a create_session_globals_then gets nested within another one.

You can see test files like compiler/rustc_expand/src/parse/tests.rs have multiple tests, each one containing a create_default_session_globals_then call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't much like the if_not_set_then functions for this reason. They only exist because there are some places where initialization is messy and there are code paths that are sometimes executed within a create_session_globals_then and sometimes not, depending on whether it's rustc vs. rustdoc. vs. whatever else. They could probably be removed with some effort.

Copy link
Member

Choose a reason for hiding this comment

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

I see.

Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

one question, r=me on the other 8 commits

@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 3, 2023

📌 Commit 6358411 has been approved by Nilstrieb

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 Nov 3, 2023
@bors
Copy link
Contributor

bors commented Nov 3, 2023

⌛ Testing commit 6358411 with merge 9c20ddd...

@bors
Copy link
Contributor

bors commented Nov 3, 2023

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing 9c20ddd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 3, 2023
@bors bors merged commit 9c20ddd into rust-lang:master Nov 3, 2023
12 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Nov 3, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9c20ddd): 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.4% [0.4%, 0.4%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [0.4%, 0.4%] 2

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

Cycles

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

Binary size

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

Bootstrap: 635.543s -> 636.863s (0.21%)
Artifact size: 304.59 MiB -> 304.49 MiB (-0.04%)

@nnethercote nnethercote deleted the rustc_span branch November 3, 2023 22:03
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 6, 2023
84: Automated pull from upstream `master` r=Dajamante a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117585
* rust-lang/rust#117576
* rust-lang/rust#96979
* rust-lang/rust#117191
* rust-lang/rust#117179
* rust-lang/rust#117574
* rust-lang/rust#117537
* rust-lang/rust#117608
  * rust-lang/rust#117596
  * rust-lang/rust#117588
  * rust-lang/rust#117524
  * rust-lang/rust#116017
* rust-lang/rust#117504
* rust-lang/rust#117469
* rust-lang/rust#116218
* rust-lang/rust#117589
* rust-lang/rust#117581
* rust-lang/rust#117503
* rust-lang/rust#117590
  * rust-lang/rust#117583
  * rust-lang/rust#117570
  * rust-lang/rust#117562
  * rust-lang/rust#117534
  * rust-lang/rust#116894
  * rust-lang/rust#110340
* rust-lang/rust#113343
* rust-lang/rust#117579
* rust-lang/rust#117094
* rust-lang/rust#117566
* rust-lang/rust#117564
  * rust-lang/rust#117554
  * rust-lang/rust#117550
  * rust-lang/rust#117343
* rust-lang/rust#115274
* rust-lang/rust#117540
* rust-lang/rust#116412
* rust-lang/rust#115333
* rust-lang/rust#117507
* rust-lang/rust#117538
  * rust-lang/rust#117533
  * rust-lang/rust#117523
  * rust-lang/rust#117520
  * rust-lang/rust#117505
  * rust-lang/rust#117434
* rust-lang/rust#117535
* rust-lang/rust#117510
* rust-lang/rust#116439
* rust-lang/rust#117508



Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Co-authored-by: SabrinaJewson <sejewson@gmail.com>
Co-authored-by: J-ZhengLi <lizheng135@huawei.com>
Co-authored-by: koka <koka.code@gmail.com>
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
Co-authored-by: lengyijun <sjtu5140809011@gmail.com>
Co-authored-by: Zalathar <Zalathar@users.noreply.github.com>
Co-authored-by: Oli Scherer <git-spam-no-reply9815368754983@oli-obk.de>
Co-authored-by: Philipp Krones <hello@philkrones.com>
Co-authored-by: y21 <30553356+y21@users.noreply.github.com>
Co-authored-by: bors <bors@rust-lang.org>
Co-authored-by: bohan <bohan-zhang@foxmail.com>
zhassan-aws added a commit to model-checking/kani that referenced this pull request Nov 6, 2023
Update to the latest Rust toolchain (2023-11-06).

The relevant changes are:
- rust-lang/rust#117507: this required changing
the import of `Span` from `rustc_span::source_map::Span` to
`rustc_span::Span`.
- rust-lang/rust#114208: this changed the data
field for the `OffsetOf` variant of `NullOp` from `List<FieldIdx>` to
`List<(VariantIdx, FieldIdx)>`, which required updating the relevant
code in `rvalue.rs`.
- rust-lang/rust#115626: the unchecked shift
operators have been separated from the `unchecked_math` feature, so this
required changing the feature annotation in
`tests/ui/should-panic-attribute/unexpected-failures/test.rs`
- Some rustc change (not sure which one) result in a line in
`tests/coverage/unreachable/variant/main.rs` getting optimized out. To
maintain what this test is testing, I changed the `match` to make it a
bit less-prone to optimization.
- A change in `cargo` (rust-lang/cargo#12779)
resulted in an update to Kani's workspace `Cargo.toml` when `cargo add`
is executed inside `tests/script-based-pre/build-cache-bin`. This is
apparently intended behavior, so I had to make the `exclude` in the
`Cargo.toml` more specific to make sure this doesn't happen (I tried
using a glob, but that didn't work, apparently because of
rust-lang/cargo#6009.

Resolves #2848 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 16, 2023
`rustc_span` cleanups

Just some things I found while looking over this crate.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants