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.

Sign up for GitHub

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

Jump to bottom

ParseSess cleanups #121969

Merged
merged 5 commits into from Mar 5, 2024
Merged

ParseSess cleanups #121969

merged 5 commits into from Mar 5, 2024

Conversation

nnethercote
Copy link
Contributor

The main change here is to rename all ParseSess values as psess. Plus a few other small cleanups.

r? @wesleywiser

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

rustbot commented Mar 4, 2024

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@nnethercote
Copy link
Contributor Author

Best reviewed one commit at a time. Each commit log has the relevant details.

There is an argument to be made that Session should be renamed Sess, to match ParseSess and all the values named sess, but I haven't done that here.

A question that you might be wondering: why do we even need the Session/ParseSess split? Could they be merged? The short answer is "not really". To do any parsing, you need a ParseSess, and there are a lot of places that need to do at least a little bit of parsing. A Session contains a ParseSess and quite a few other things; it's a bigger and heavier piece of state, and fewer places need all that.

My gut feeling is that the psess renaming doesn't quite reach the level of needing an MCP.

@wesleywiser
Copy link
Member

I think personally I would have gone with parse_sess but I don't have an objection to psess and you've already done the work 🙂

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 4, 2024

📌 Commit 287fcde has been approved by wesleywiser

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 Mar 4, 2024
calebcartwright
calebcartwright reviewed Mar 4, 2024
View reviewed changes
src/tools/rustfmt/src/parse/session.rs Outdated
@@ -23,7 +23,7 @@ use crate::{Config, ErrorKind, FileName};

/// ParseSess holds structs necessary for constructing a parser.
pub(crate) struct ParseSess {
parse_sess: RawParseSess,
raw_psess: RawParseSess, // njn: hmm
Copy link
Member

Choose a reason for hiding this comment

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

no issues with this, but I did chuckle a bit as I can imagine seeing this again in a couple months/years and saying "hmmm" myself in response to the comment 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that isn't supposed to be there :) I use njn comments (my initials) to indicate things I need to reconsider/fix up before I post the PR. I wasn't sure about this field name at first but now I think it's fine. I have now updated and removed the comment.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2024
@matthiaskrgr
Rollup merge of rust-lang#121969 - nnethercote:ParseSess-cleanups, r=…
99c4935 
…wesleywiser

`ParseSess` cleanups

The main change here is to rename all `ParseSess` values as `psess`. Plus a few other small cleanups.

r? `@wesleywiser`
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 4, 2024
Closed
@nnethercote
Rename all ParseSess variables/fields/lifetimes as psess.
80d2bdb 
Existing names for values of this type are `sess`, `parse_sess`,
`parse_session`, and `ps`. `sess` is particularly annoying because
that's also used for `Session` values, which are often co-located, and
it can be difficult to know which type a value named `sess` refers to.
(That annoyance is the main motivation for this change.) `psess` is nice
and short, which is good for a name used this much.

The commit also renames some `parse_sess_created` values as
`psess_created`.
@nnethercote
Copy link
Contributor Author

Oh, I just force-pushed a comment fix before I saw this was part of a rollup, sorry.

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Mar 4, 2024

📌 Commit 80d2bdb has been approved by wesleywiser

It is now in the queue for this repository.

@matthiaskrgr matthiaskrgr mentioned this pull request Mar 4, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2024
@bors
Auto merge of rust-lang#121998 - matthiaskrgr:rollup-l7lzwpb, r=matth…
6710082 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#120976 (constify a couple thread_local statics)
 - rust-lang#121683 (Fix LVI tests after frame pointers are enabled by default)
 - rust-lang#121703 (Add a way to add constructors for `rustc_type_ir` types)
 - rust-lang#121732 (Improve assert_matches! documentation)
 - rust-lang#121928 (Extract an arguments struct for `Builder::then_else_break`)
 - rust-lang#121939 (Small enhancement to description of From trait)
 - rust-lang#121968 (Don't run test_get_os_named_thread on win7)
 - rust-lang#121969 (`ParseSess` cleanups)
 - rust-lang#121977 (Doc: Fix incorrect reference to integer in Atomic{Ptr,Bool}::as_ptr.)
 - rust-lang#121994 (Update platform-support.md with supported musl version)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2024
@bors
Auto merge of rust-lang#121998 - matthiaskrgr:rollup-l7lzwpb, r=matth…
50e77f1 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#120976 (constify a couple thread_local statics)
 - rust-lang#121683 (Fix LVI tests after frame pointers are enabled by default)
 - rust-lang#121703 (Add a way to add constructors for `rustc_type_ir` types)
 - rust-lang#121732 (Improve assert_matches! documentation)
 - rust-lang#121928 (Extract an arguments struct for `Builder::then_else_break`)
 - rust-lang#121939 (Small enhancement to description of From trait)
 - rust-lang#121968 (Don't run test_get_os_named_thread on win7)
 - rust-lang#121969 (`ParseSess` cleanups)
 - rust-lang#121977 (Doc: Fix incorrect reference to integer in Atomic{Ptr,Bool}::as_ptr.)
 - rust-lang#121994 (Update platform-support.md with supported musl version)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 13b9712 into rust-lang:master Mar 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
@rust-timer
Unrolled build for rust-lang#121969
96b9ffc 
Rollup merge of rust-lang#121969 - nnethercote:ParseSess-cleanups, r=wesleywiser

`ParseSess` cleanups

The main change here is to rename all `ParseSess` values as `psess`. Plus a few other small cleanups.

r? `@wesleywiser`
@nnethercote nnethercote deleted the ParseSess-cleanups branch March 5, 2024 01:12
@smoelius smoelius mentioned this pull request Mar 6, 2024
Closed
@zhassan-aws zhassan-aws mentioned this pull request Mar 11, 2024
Merged
celinval pushed a commit to model-checking/kani that referenced this pull request Mar 12, 2024
@zhassan-aws
Upgrade toolchain to 2024-03-11 (#3071)
e7a4d83 
Relevant upstream changes:

rust-lang/rust#120675: An intrinsic `Symbol` is
now wrapped in a `IntrinsicDef` struct, so the relevant part of the code
needed to be updated.
rust-lang/rust#121464: The second argument of
the `create_wrapper_file` function changed from a vector to a string.
rust-lang/rust#121662: `NullOp::DebugAssertions`
was renamed to `NullOp::UbCheck` and it now has data (currently unused
by Kani)
rust-lang/rust#121728: Introduces `F16` and
`F128`, so needed to add stubs for them
rust-lang/rust#121969: `parse_sess` was renamed
to `psess`, so updated the relevant code.
rust-lang/rust#122059: The
`is_val_statically_known` intrinsic is now used in some `core::fmt`
code, so had to handle it in (codegen'ed to false).
rust-lang/rust#122233: This added a new
`retag_box_to_raw` intrinsic. This is an operation that is primarily
relevant for stacked borrows. For Kani, we just return the pointer.

Resolves #3057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calebcartwright calebcartwright calebcartwright left review comments

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nnethercote @rustbot @wesleywiser @bors @calebcartwright