Pass the crate environment to proc macros #6820

jonas-schievink · 2020-12-11T14:01:12Z

In theory, fixes #6696.

This seems to result in these obscure crashes for some reason:

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/scoped_cell.rs:75:51
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:483
   1: core::panicking::panic_fmt
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/panicking.rs:85
   2: core::panicking::panic
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/panicking.rs:50
   3: core::option::Option<T>::unwrap
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:383
   4: proc_macro::bridge::scoped_cell::ScopedCell<T>::replace
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/scoped_cell.rs:75
   5: proc_macro::bridge::client::BridgeState::with::{{closure}}
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:291
   6: std::thread::local::LocalKey<T>::try_with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:272
   7: std::thread::local::LocalKey<T>::with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:248
   8: proc_macro::bridge::client::BridgeState::with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:290
   9: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:329
  10: proc_macro::bridge::client::run_client::{{closure}}::{{closure}}
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:375
  11: proc_macro::bridge::scoped_cell::ScopedCell<T>::set::{{closure}}
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/scoped_cell.rs:80
  12: proc_macro::bridge::scoped_cell::ScopedCell<T>::replace
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/scoped_cell.rs:75
  13: proc_macro::bridge::scoped_cell::ScopedCell<T>::set
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/scoped_cell.rs:80
  14: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter::{{closure}}
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:325
  15: std::thread::local::LocalKey<T>::try_with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:272
  16: std::thread::local::LocalKey<T>::with
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:248
  17: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:325
  18: proc_macro::bridge::client::run_client::{{closure}}
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:370
  19: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:308
  20: std::panicking::try::do_call
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:381
  21: __rust_try
  22: std::panicking::try
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:345
  23: std::panic::catch_unwind
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:382
  24: proc_macro::bridge::client::run_client
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:369
  25: proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>::expand1::run
             at /home/jonas/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/bridge/client.rs:410
  26: proc_macro_srv::proc_macro::bridge::server::run_server
  27: proc_macro_srv::dylib::Expander::expand
  28: proc_macro_srv::ProcMacroSrv::expand
  29: proc_macro_srv::cli::run
  30: rust_analyzer::main

matklad

for some reason:

Does it makes sense to investigate further, to figure out what is the problem?

matklad · 2020-12-11T14:09:33Z

crates/proc_macro_api/src/rpc.rs

@@ -51,6 +51,9 @@ pub struct ExpansionTask {
    pub attributes: Option<Subtree>,

    pub lib: PathBuf,
+
+    /// Environment variables to set during macro expansion.
+    pub env: Vec<(String, String)>,


Would OsString be a better fit here semantic-wise?

In analysis, we need to use Strings, because only that's compatible with env!.

Here, the consumre is external process, so OsString might be better?

But mostly, this is irrelevant.

Yeah, that's true. But they come from Env, so we won't see non-UTF-8 variables.

Yeah, in similar cases I generally prefer to shape the interface to the shape of the external world, rather than to the shape of an internal impl.

It's less confusing to have some explicit glue code, than for two things share identical inteface by accident.

matklad · 2020-12-11T14:11:15Z

crates/base_db/src/input.rs

+        &self,
+        subtree: &Subtree,
+        attrs: Option<&Subtree>,
+        env: &Env,


Hm, yeah, I think this makes sense -- proc macro can observe compile-time env variables.

jonas-schievink · 2020-12-11T14:32:58Z

Does it makes sense to investigate further, to figure out what is the problem?

Maybe, yeah. It would be good to have more people familiar with how the proc macro server works, but it is very hard to understand.

edwin0cheng · 2020-12-15T10:25:06Z

I am not familiar with how the proc macro server works either, but would like to help to find the problem. How could I reproduce the panic in description ?

jonas-schievink · 2020-12-15T11:57:57Z

I've tried to use frame-support, as mentioned here: #6696 (comment)

TheRawMeatball · 2020-12-21T11:10:44Z

Why is this a draft? I have this branch installed and its been really helpful.

edwin0cheng · 2020-12-21T11:14:15Z

Because we (and I) are still investigate how the panic is raised in the #6820 (comment)

@jonas-schievink

7047: Add force_show_panics flag for proc-macro bridge r=jonas-schievink a=edwin0cheng rust-lang/rust#75082 and rust-lang/rust#76292 added a new flag in `proc_macro::Bridge` such that the ABI was changed. These ABI changing are the reason of some weird panics which caused #6880 and maybe related to the panic mentioned in #6820. These changes are landed on rust stable 1.48 so I think it is okay to apply it now. fixes #6880 r @jonas-schievink Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>

edwin0cheng · 2020-12-27T13:39:46Z

#7047 should be the cause of that panic and I think this PR could be okay now ?

It's only used to break the dependency to proc_macro_api

jonas-schievink · 2020-12-27T14:38:46Z

Tested locally, this seems to work fine now!

bors r+

bors · 2020-12-27T14:45:49Z

Build succeeded:

matklad reviewed Dec 11, 2020

View reviewed changes

bjorn3 mentioned this pull request Dec 15, 2020

CARGO_MANIFEST_DIR must be set and could not resolve macro json_internal #6890

Closed

edwin0cheng mentioned this pull request Dec 27, 2020

Add force_show_panics flag for proc-macro bridge #7047

Merged

jonas-schievink added 2 commits December 27, 2020 15:29

Move TokenExpander to base_db and rename it

798968e

It's only used to break the dependency to proc_macro_api

Pass crate environment to proc macros

7087742

jonas-schievink marked this pull request as ready for review December 27, 2020 14:37

bors bot merged commit 8f26221 into rust-lang:master Dec 27, 2020

edwin0cheng mentioned this pull request Jan 7, 2021

Proc macros are executed in the wrong working directory #7057

Closed

jonas-schievink deleted the proc-macro-env branch April 7, 2021 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass the crate environment to proc macros #6820

Pass the crate environment to proc macros #6820

jonas-schievink commented Dec 11, 2020 •

edited

Loading

matklad left a comment

matklad Dec 11, 2020

jonas-schievink Dec 11, 2020

matklad Dec 14, 2020

matklad Dec 11, 2020

jonas-schievink commented Dec 11, 2020

edwin0cheng commented Dec 15, 2020

jonas-schievink commented Dec 15, 2020

TheRawMeatball commented Dec 21, 2020

edwin0cheng commented Dec 21, 2020 •

edited

Loading

edwin0cheng commented Dec 27, 2020

jonas-schievink commented Dec 27, 2020

bors bot commented Dec 27, 2020

Pass the crate environment to proc macros #6820

Pass the crate environment to proc macros #6820

Conversation

jonas-schievink commented Dec 11, 2020 • edited Loading

matklad left a comment

Choose a reason for hiding this comment

matklad Dec 11, 2020

Choose a reason for hiding this comment

jonas-schievink Dec 11, 2020

Choose a reason for hiding this comment

matklad Dec 14, 2020

Choose a reason for hiding this comment

matklad Dec 11, 2020

Choose a reason for hiding this comment

jonas-schievink commented Dec 11, 2020

edwin0cheng commented Dec 15, 2020

jonas-schievink commented Dec 15, 2020

TheRawMeatball commented Dec 21, 2020

edwin0cheng commented Dec 21, 2020 • edited Loading

edwin0cheng commented Dec 27, 2020

jonas-schievink commented Dec 27, 2020

bors bot commented Dec 27, 2020

jonas-schievink commented Dec 11, 2020 •

edited

Loading

edwin0cheng commented Dec 21, 2020 •

edited

Loading