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

[WIP] rustc_mir: disallow global mutable state in proc macros. #63831

Open
wants to merge 1 commit into
base: master
from

Conversation

@eddyb
Copy link
Member

commented Aug 23, 2019

Along the lines of #63809, this PR attempts to get rid of the main (or only?) place proc_macro handles could be leaked to, and further disallow/discourage sharing (other) state between invocations.

The approach of banning (interior-)mutable statics was most recently mentioned in #63804 (comment), but it's likely been brought up several times, we just never tried it.
(Note that this is not foolproof: one would have to scan all dependencies for such statics, modulo proc_macro/std, and even then it's possible there would be a lot of false positives)

So this is mostly for a check-only crater run, to see what (if anything) breaks.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Aug 23, 2019

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@eddyb

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2019

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

⌛️ Trying commit ee8492f with merge 8c9c32a...

bors added a commit that referenced this pull request Aug 23, 2019
Auto merge of #63831 - eddyb:proc-macro-statecheck, r=<try>
[WIP] rustc_mir: disallow global mutable state in proc macros.

Along the lines of #63809, this PR attempts to get rid of the main (or only?) place `proc_macro` handles could be leaked to, *and* further disallow/discourage sharing (other) state between invocations.

The approach of banning (interior-)mutable `static`s was most recently mentioned in #63804 (comment), but it's likely been brought up several times, we just never tried it.
(Note that this is not foolproof: one would have to scan all dependencies for such `static`s, modulo `proc_macro`/`std`, and even then it's possible there would be a lot of false positives)

So this is mostly for a check-only crater run, to see what (if anything) breaks.
@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

☀️ Try build successful - checks-azure
Build commit: 8c9c32a

@eddyb

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2019

@craterbot run mode=check-only

@eddyb

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2019

@craterbot run mode=check-only

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented Aug 27, 2019

👌 Experiment pr-63831 created and queued.
🤖 Automatically detected try build 8c9c32a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@pietroalbini

This comment has been minimized.

Copy link
Member

commented Sep 8, 2019

@craterbot crates=full

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented Sep 8, 2019

📝 Configuration of the pr-63831 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@eddyb eddyb force-pushed the eddyb:proc-macro-statecheck branch from ee8492f to 55f3acf Sep 12, 2019

bors added a commit that referenced this pull request Sep 12, 2019
Auto merge of #64398 - eddyb:crater-63809-63812-63831, r=<try>
[WIP] Crater rollup of 3 PRs (#63809, #63812, #63831).

I've combined my 3 PRs in the crater queue:
* #63809 proc_macro: check non-interned handles for "leaks" between/after invocations.
* #63812 rustc_mir: double-check const-promotion candidates for sanity.
* #63831 rustc_mir: disallow global mutable state in proc macros.

cc @pietroalbini @Mark-Simulacrum Is this a good idea?
@eddyb

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

@craterbot abort
(in favor of #64398)

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2019

🗑 Experiment pr-63831 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.