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

Rework SESSION_GLOBALS API #84961

Merged
merged 2 commits into from
Jul 8, 2021
Merged

Rework SESSION_GLOBALS API #84961

merged 2 commits into from
Jul 8, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 5, 2021 •
edited
Loading

Fixes #84954.

Needs #84953 to be merged first (I cherry-picked its commits to have CI pass). (done)

r? @Aaron1011

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from a539fa4 to 832fdbb Compare May 5, 2021 20:25
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 5, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch 3 times, most recently from adb0f3c to 1baea29 Compare May 5, 2021 21:09
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 5, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from 1baea29 to 64849f5 Compare May 5, 2021 21:24
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 5, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from 64849f5 to d84e497 Compare May 5, 2021 21:37
@Aaron1011 Aaron1011 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 6, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from d84e497 to e90cc20 Compare May 6, 2021 08:28
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 6, 2021
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 6, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from e90cc20 to 9af7e5f Compare May 6, 2021 08:44
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member Author

Funny bug in the compiler error output (use rustc_span::symbol::Ident; is only present once):

Diff in /checkout/compiler/rustc_ast_pretty/src/pprust/tests.rs at line 1:
 use super::*;
 use rustc_ast as ast;
-use rustc_span::symbol::Ident;
-use rustc_span::symbol::Ident;

@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch 3 times, most recently from 505b5c4 to 2cf8887 Compare May 6, 2021 12:25
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 6, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from 2cf8887 to 3063c74 Compare May 6, 2021 12:38
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 6, 2021
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from 3063c74 to 1e00e9d Compare May 6, 2021 14:14
@rust-lang rust-lang deleted a comment from rust-log-analyzer May 6, 2021
@GuillaumeGomez
Copy link
Member Author

I need some help to try to understand what's going on with rustfmt.

@bstrie
Copy link
Contributor

bstrie commented Jun 2, 2021

Triage here, any progress? Ordinarily I would reset the status to waiting-on-author, but I see that you manually set it to waiting-on-review.

@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from 6634d70 to e05ee1f Compare June 10, 2021 19:10
@GuillaumeGomez
Copy link
Member Author

The PR is now ready for review! \o/

@GuillaumeGomez
Copy link
Member Author

@Aaron1011 doesn't seem to be around so let's assign someone else.

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned Aaron1011 Jun 18, 2021
@GuillaumeGomez GuillaumeGomez added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 19, 2021
@GuillaumeGomez
Copy link
Member Author

Or anyone else from the @rust-lang/compiler team?

@pnkfelix
Copy link
Member

pnkfelix commented Jul 8, 2021

This seems fine.

The most important thing, AFAICT, is the new assertions that have been added to ensure that the session globals are not newly created (i.e. overwritten) once they have been established on a given thread.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 8, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 8, 2021

📌 Commit e05ee1f has been approved by pnkfelix

@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 Jul 8, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 8, 2021
@GuillaumeGomez
Rollup merge of rust-lang#84961 - GuillaumeGomez:rework-session-globa…
82e7f7e 
…ls, r=pnkfelix

Rework SESSION_GLOBALS API

Fixes rust-lang#84954.

<s>Needs rust-lang#84953 to be merged first (I cherry-picked its commits to have CI pass).</s> (done)

r? `@Aaron1011`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 8, 2021
Closed
@GuillaumeGomez GuillaumeGomez force-pushed the rework-session-globals branch from e05ee1f to a2654fb Compare July 8, 2021 14:36
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jul 8, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2021

📌 Commit d891c8c has been approved by oli-obk

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 8, 2021
@GuillaumeGomez
Rollup merge of rust-lang#84961 - GuillaumeGomez:rework-session-globa…
a20f0d7 
…ls, r=oli-obk

Rework SESSION_GLOBALS API

Fixes rust-lang#84954.

<s>Needs rust-lang#84953 to be merged first (I cherry-picked its commits to have CI pass).</s> (done)

r? `@Aaron1011`
This was referenced Jul 8, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2021
@bors
Auto merge of rust-lang#86982 - GuillaumeGomez:rollup-7sbye3c, r=Guil…
aa65b08 
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#84961 (Rework SESSION_GLOBALS API)
 - rust-lang#86726 (Use diagnostic items instead of lang items for rfc2229 migrations)
 - rust-lang#86789 (Update BTreeSet::drain_filter documentation)
 - rust-lang#86838 (Checking that function is const if marked with rustc_const_unstable)
 - rust-lang#86903 (Fix small headers display)
 - rust-lang#86913 (Document rustdoc with `--document-private-items`)
 - rust-lang#86957 (Update .mailmap file)
 - rust-lang#86971 (mailmap: Add alternative addresses for myself)

Failed merges:

 - rust-lang#86869 (Account for capture kind in auto traits migration)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d9297ae into rust-lang:master Jul 8, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 8, 2021
@GuillaumeGomez GuillaumeGomez deleted the rework-session-globals branch July 8, 2021 20:24
JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Jul 15, 2021
@JohnTitor
Rustup to rust-lang/rust#84961
251d0f1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aaron1011 Aaron1011 Aaron1011 left review comments

Assignees

@pnkfelix pnkfelix

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
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.

Don't allow overwriting SESSION_GLOBALS
9 participants
@GuillaumeGomez @rust-log-analyzer @bors @Aaron1011 @bstrie @pnkfelix @oli-obk @rust-highfive @rustbot