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

Move SourceMap initialization #122811

Merged
merged 3 commits into from
Apr 16, 2024
Merged

Conversation

nnethercote
Copy link
Contributor

So it happens at the same time as SessionGlobals initialization, rather than shortly afterward.

r? @WaffleLapkin

@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. labels Mar 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2024

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@nnethercote
Copy link
Contributor Author

This is a follow-up to #122810, and the first two commits come from that PR. I've put this in a separate PR so we can merge the removal of CodegenBackend::target_override first, and see if that causes any problems before making changes that build on that.

Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

This mostly looks good. I have a couple comments, but we'll have to wait for #122811 to merge first anyway.

compiler/rustc_interface/src/tests.rs Outdated Show resolved Hide resolved
compiler/rustc_span/src/lib.rs Show resolved Hide resolved
compiler/rustc_span/src/source_map.rs Outdated Show resolved Hide resolved
@nnethercote
Copy link
Contributor Author

I have updated and addressed the comments. I think this shouldn't land quite yet, to give a little more time for any fallout from #122810 to happen. I will be around on Monday but then I will be on vacation for three weeks. Maybe this should wait until I get back, just to be safe.

@WaffleLapkin
Copy link
Member

r=me whenever you feel like we waited enough :)

@bors
Copy link
Contributor

bors commented Mar 28, 2024

☔ The latest upstream changes (presumably #115220) made this pull request unmergeable. Please resolve the merge conflicts.

Replacing `mk_session` with `sess_and_cfgs`, which does a bit more of
the shared stuff -- the option parsing and the `build_configuration`
call.
Currently `SourceMap` is constructed slightly later than
`SessionGlobals`, and inserted. This commit changes things so they are
done at the same time.

Benefits:
- `SessionGlobals::source_map` changes from
  `Lock<Option<Lrc<SourceMap>>>` to `Option<Lrc<SourceMap>>`. It's still
  optional, but mutability isn't required because it's initialized at
  construction.
- `set_source_map` is removed, simplifying `run_compiler`, which is
  good because that's a critical function and it's nice to make it
  simpler.

This requires moving things around a bit, so the necessary inputs are
available when `SessionGlobals` is created, in particular the `loader`
and `hash_kind`, which are no longer computed by `build_session`. These
inputs are captured by the new `SourceMapInputs` type, which is threaded
through various places.
Currently it's a method on `EarlyDiagCtxt`, which is not the right place
for it at all -- `EarlyDiagCtxt` is used to issue diagnostics, but
shouldn't be doing any of the actual checking.

This commit moves it into a standalone function that takes an
`EarlyDiagCtxt` as an argument, which is more sensible. This does
require adding `EarlyDiagCtxt::early_struct_warn`, so a warning can be
returned and then modified with a note. (And that likely explains why
somebody put `initialize_checked_jobserver` into `EarlyDiagCtxt` in the
first place.)
@nnethercote
Copy link
Contributor Author

There have been no complaints about #122810. I think this is safe to merge.

@bors r=WaffleLapkin

@bors
Copy link
Contributor

bors commented Apr 16, 2024

📌 Commit 9b0ced0 has been approved by WaffleLapkin

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 Apr 16, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122811 (Move `SourceMap` initialization)
 - rust-lang#123512 (Match ergonomics 2024: Implement eat-one-layer)
 - rust-lang#123811 (Use queue-based `RwLock` on more platforms)
 - rust-lang#123859 (Remove uneeded clones now that TrustedStep implies Copy)
 - rust-lang#123979 (Subtype predicates only exist on inference types, so we can allow them to register opaque types within them.)
 - rust-lang#124016 (Outline default query and hook provider function implementations)
 - rust-lang#124023 (Allow workproducts without object files.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 14496d5 into rust-lang:master Apr 16, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
Rollup merge of rust-lang#122811 - nnethercote:mv-SourceMap-init, r=WaffleLapkin

Move `SourceMap` initialization

So it happens at the same time as `SessionGlobals` initialization, rather than shortly afterward.

r? `@WaffleLapkin`
@nnethercote nnethercote deleted the mv-SourceMap-init branch April 16, 2024 22:07
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 18, 2024
…affleLapkin

Move `SourceMap` initialization

So it happens at the same time as `SessionGlobals` initialization, rather than shortly afterward.

r? `@WaffleLapkin`
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants