Skip to content

Move REGISTERED_DIAGNOSTICS to a ParseSess field - #48808

Merged
bors merged 2 commits into
rust-lang:masterfrom
Zoxc:reg-diag
Mar 9, 2018
Merged

Move REGISTERED_DIAGNOSTICS to a ParseSess field#48808
bors merged 2 commits into
rust-lang:masterfrom
Zoxc:reg-diag

Conversation

@Zoxc

@ZoxcZoxc commented Mar 7, 2018

Copy link
Copy Markdown
Contributor

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2018
michaelwoerister
michaelwoerister reviewed Mar 7, 2018
View reviewed changes

@michaelwoeristermichaelwoerister left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. However, I think it would really be an improvement if we had a Lock::with() method as that would encourage making locking ranges more visible and explicit.

Comment threadsrc/libsyntax/diagnostics/plugin.rs Outdated

@michaelwoeristermichaelwoeristerMar 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we add a with method to Lock? That would make it clearer in many cases whether the lock is held until the end of the block intentionally or not.

@ZoxcZoxcMar 7, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's probably not a bad idea. with sounds a little innocent for a lock though.

@michaelwoeristermichaelwoeristerMar 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

with_lock() maybe?

@Zoxc

Zoxc commented Mar 8, 2018

Copy link
Copy Markdown
ContributorAuthor

I added methods to lock in a closure.

@michaelwoerister

michaelwoerister commented Mar 8, 2018

Copy link
Copy Markdown
Member

👍

@bors r+

@bors

bors commented Mar 8, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 728c16c has been approved by michaelwoerister

@borsbors 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 8, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 8, 2018
@Manishearth
Rollup merge ofrust-lang#48808- Zoxc:reg-diag, r=michaelwoerister
4579753
Move REGISTERED_DIAGNOSTICS to a ParseSess field
r? @michaelwoerister
@ManishearthManishearth mentioned this pull request Mar 8, 2018
Merged
bors added a commit that referenced this pull request Mar 8, 2018
@bors
Auto merge of#48849- Manishearth:rollup, r=Manishearth
604d4ce
Rollup of 7 pull requests
- Successful merges: #48292, #48682, #48699, #48738, #48752, #48789, #48808
- Failed merges:
@bors
bors merged commit 728c16c into rust-lang:masterMar 9, 2018
@Zoxc
Zoxc deleted the reg-diag branch March 14, 2018 16:38
@aturonaturon mentioned this pull request Aug 16, 2019
Closed
37 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@michaelwoeristermichaelwoeristermichaelwoerister left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelwoeristermichaelwoerister

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Zoxc@michaelwoerister@bors@rust-highfive