Cache strategy hits assertion in Rust-Analyzer

[djc]

After my weekly dependency updates this morning, I'm having async-graphql macros failing in my crates with this assertion:

thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `"/Users/djc/src/bolt/rust/domains/epoxide"`,
 right: `"/Users/djc/src/bolt/rust/domains/store"`: CARGO_MANIFEST_DIR must not change within one compiler process', /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro-crate-1.2.0/src/lib.rs:137:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panicking.rs:142:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panicking.rs:181:5
   4: proc_macro_crate::crate_name
             at /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro-crate-1.2.0/src/lib.rs:137:5
   5: async_graphql_derive::utils::get_crate_name
             at /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/async-graphql-derive-4.0.6/src/utils.rs:40:26
   6: async_graphql_derive::simple_object::generate
             at /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/async-graphql-derive-4.0.6/src/simple_object.rs:29:22
   7: async_graphql_derive::derive_simple_object
             at /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/async-graphql-derive-4.0.6/src/lib.rs:52:11
   8: core::ops::function::FnOnce::call_once
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/ops/function.rs:248:5
   9: proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>::expand1::run::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:424:40
  10: proc_macro::bridge::client::run_client::{{closure}}::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:392:26
  11: proc_macro::bridge::scoped_cell::ScopedCell<T>::set::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/scoped_cell.rs:79:33
  12: proc_macro::bridge::scoped_cell::ScopedCell<T>::replace
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/scoped_cell.rs:74:9
  13: proc_macro::bridge::scoped_cell::ScopedCell<T>::set
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/scoped_cell.rs:79:9
  14: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:340:35
  15: std::thread::local::LocalKey<T>::try_with
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/local.rs:445:16
  16: std::thread::local::LocalKey<T>::with
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/local.rs:421:9
  17: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:340:9
  18: proc_macro::bridge::client::run_client::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:385:9
  19: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panic/unwind_safe.rs:271:9
  20: std::panicking::try::do_call
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:492:40
  21: ___rust_try
  22: std::panicking::try
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:456:19
  23: std::panic::catch_unwind
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panic.rs:137:14
  24: proc_macro::bridge::client::run_client
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:384:5
  25: proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>::expand1::run
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/proc_macro/src/bridge/client.rs:424:13
  26: proc_macro_srv::abis::abi_1_58::proc_macro::bridge::server::run_server
  27: proc_macro_srv::abis::abi_1_58::Abi::expand
  28: proc_macro_srv::dylib::Expander::expand
  29: core::ops::function::FnOnce::call_once{{vtable.shim}}

It looks like #20 is making assumptions it should not be making. Rust-Analyzer issue: https://github.com/rust-lang/rust-analyzer/issues/12969.

cc @jplatte

[lnicola]

To clarify, the assumption might be valid, or is at least reasonable given that it works fine in rustc. It's just that we don't really unload the proc macro libraries, and use a single process per Cargo workspace, IIRC. We could probably spawn a new process for each proc macro, but I wouldn't be surprised if users started complaining about RA being a resource hog with dozens of processes.

A quick workaround on your side might be to store that value in a thread-local variable. We currently expand each macro call in a new thread (because the bridge now requires that), so we wouldn't get the benefits of the cache, but at least it would work.

[jplatte]

This should be easy to fix / work around in proc-macro-crate so I'll work on a PR. I'm also very interested in what others think about this assumption though. I guess this is the wrong place for that discussion, maybe https://github.com/rust-lang/rust-analyzer/issues/12969 is right.

[bjorn3]

Turning the CACHE static at https://github.com/bkchr/proc-macro-crate/blob/66213a5d49a46f4986a5b2d9cee0cbefab5e917b/src/lib.rs#L118 into a thread local should fix this as thread locals are cleared between invocations in rust-analyzer as it puts every invocation in it's own thread. It shouldn't regress performance in rustc as rustc uses a single thread right now. This will probably change in the future though. Another option would be to make it a map from CARGO_MANIFEST_DIR to crate name. This will keep using the old name if the crate name gets changed in Cargo.toml though.

[jplatte]

Another option would be to make it a map from CARGO_MANIFEST_DIR to crate name. This will keep using the old name if the crate name gets changed in Cargo.toml though.

This is what I'm doing, and I'm also adding a timestamp based on fs metadata so changing the Cargo.toml file and re-running the proc macro from the same process won't use outdated information from the cache.