More thread-safety changes#49390

Zoxc · 2018-03-26T19:36:22Z

michaelwoerister · 2018-03-27T12:16:34Z

@rust-lang/compiler Does anybody know what LazyTokenStream is? It seems to be unused.

petrochenkov · 2018-03-27T17:52:29Z

Does anybody know what LazyTokenStream is?

It's a hack! But it's used.
Many nonterminal tokens (e.g. NtExpr) don't know their original token streams from which they were parsed (they have only AST fragments like ast::Expr), but these token streams are sometimes needed (for example if they are a part of macro definition and that macro definition needs to be stored in metadata), so AST fragments are pretty-printed into text (!) and then parsed back into the token stream (on best-effort basis).
LazyTokenStream serves as a cache for these AST -> TokenStream conversions to avoid doing these conversions multiple times, so removing it won't affect correctness, only performance.

Zoxc · 2018-03-28T06:09:16Z

@petrochenkov Why do we keep the AST around in the token stream though? It seems more correct to use the original TokenStream instead of relying on an lossy AST -> TokenStream transformation

petrochenkov · 2018-03-28T07:45:36Z

@Zoxc

Why do we keep the AST around in the token stream though?

Who knows. Nonterminal is represented with AST fragments since at least 2012, years before token streams were introduced.
Someone needs to port expansion code working with nonterminals to token trees.

shepmaster · 2018-04-07T19:42:16Z

Ping from triage, @michaelwoerister !

michaelwoerister · 2018-04-09T10:19:02Z

src/librustc/hir/map/mod.rs

    pub fn intern_inlined_body(&self, def_id: DefId, body: Body) -> &'hir Body {
+        let mut inlined_bodies = self.inlined_bodies.borrow_mut();
+        if let Some(&b) = inlined_bodies.get(&def_id) {
+            debug_assert_eq!(&body, b);


How can this condition hold? body is on the stack and b is in the arena. Am I overlooking something?

This isn't a pointer comparison. This check ensures that we always give the same body for the same def_id to intern_inlined_body.

Oh, you're right of course :)

michaelwoerister · 2018-04-09T10:26:54Z

@Zoxc, looks good to me except for intern_inlined_body. I think you can just remove the assertion there.

michaelwoerister · 2018-04-10T08:37:13Z

@bors r+

bors · 2018-04-10T08:37:14Z

📌 Commit 1e3f638 has been approved by michaelwoerister

bors · 2018-04-10T09:00:35Z

⌛ Testing commit 1e3f638 with merge 67712d7...

@michaelwoerister

More thread-safety changes r? @michaelwoerister

bors · 2018-04-10T11:44:55Z

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 67712d7 to master...

Zoxc force-pushed the sync-syntax branch from f0372bb to f679906 Compare March 26, 2018 21:04

emilyalbini assigned michaelwoerister Mar 27, 2018

emilyalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2018

Zoxc added 10 commits March 28, 2018 01:27

Make LazyTokenStream thread-safe

11ccc4c

Make SyntaxExtension thread-safe

3f0cb8c

Make ParseSess thread-safe

c979189

Add try_write to RwLock

962a53d

Make Steal thread-safe

5d128cd

Require lints to be Send + Sync

dc0fb52

Make HirMap thread-safe

8fd7d49

Make Mir::predecessors thread-safe

70de2e8

Use thread-safe types for interners

3773f4d

Make LLVM worker channel thread-safe

1e3f638

Zoxc force-pushed the sync-syntax branch from f679906 to 1e3f638 Compare March 27, 2018 23:28

michaelwoerister reviewed Apr 9, 2018

View reviewed changes

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 10, 2018

bors added a commit that referenced this pull request Apr 10, 2018

Auto merge of #49390 - Zoxc:sync-syntax, r=michaelwoerister

67712d7

More thread-safety changes r? @michaelwoerister

bors merged commit 1e3f638 into rust-lang:master Apr 10, 2018

bors mentioned this pull request Apr 10, 2018

Issue 48697 -- ICE in old borrowck mode #49666

Closed

Zoxc deleted the sync-syntax branch April 10, 2018 12:38

Uh oh!

Conversation

Zoxc commented Mar 26, 2018

Uh oh!

michaelwoerister commented Mar 27, 2018

Uh oh!

petrochenkov commented Mar 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zoxc commented Mar 28, 2018

Uh oh!

petrochenkov commented Mar 28, 2018

Uh oh!

shepmaster commented Apr 7, 2018

Uh oh!

michaelwoerister Apr 9, 2018

Choose a reason for hiding this comment

Uh oh!

Zoxc Apr 9, 2018

Choose a reason for hiding this comment

Uh oh!

michaelwoerister Apr 10, 2018

Choose a reason for hiding this comment

Uh oh!

michaelwoerister commented Apr 9, 2018

Uh oh!

michaelwoerister commented Apr 10, 2018

Uh oh!

bors commented Apr 10, 2018

Uh oh!

bors commented Apr 10, 2018

Uh oh!

bors commented Apr 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

petrochenkov commented Mar 27, 2018 •

edited

Loading