Skip to content

Learn cfgs enabled in build.rs #14452

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

Open
fitzgen opened this issue Mar 30, 2023 · 12 comments
Open

Learn cfgs enabled in build.rs #14452

fitzgen opened this issue Mar 30, 2023 · 12 comments
Labels
A-cargo cargo related issues C-bug Category: bug

Comments

@fitzgen
Copy link
Member

fitzgen commented Mar 30, 2023

Wasmtime enables cfg(compiler) when one of its compilers (Cranelift or, soon, Winch) is available and therefore JIT compilation is available. This cfg is enabled in build.rs when the cargo features for the underlying compilers are enabled. This is much easier and more future proof than writing out cfg(any(feature = "cranelift", feature = "winch")) all over the place, which is why things are done this way.

However, rust-analyzer does not treat code blocks guarded by #[cfg(compiler)] as enabled, and fails to do autocomplete/jump-to-def/etc within these regions, even though changing the cfg to the wordier cfg(any(...)) does work.

It would be nice if rust-analyzer looked at the cfgs defined by build.rs and enabled them for the project.

@fitzgen fitzgen added the C-feature Category: feature request label Mar 30, 2023
@Veykril Veykril added the A-cargo cargo related issues label Mar 30, 2023
@Veykril
Copy link
Member

Veykril commented Mar 30, 2023

Unsure how we would get to the output there since we have cargo execute the build scripts which will in turn eat that. I guess we could check <CRATE_OUT_DIR>/../output and parse that?

Actually, shouldn't we be doing this already? If I am not mistaken the cfg field of BuildScript is just this right?

@Veykril Veykril added C-bug Category: bug and removed C-feature Category: feature request labels Mar 30, 2023
@flodiebold
Copy link
Member

Actually, shouldn't we be doing this already? If I am not mistaken the cfg field of BuildScript is just this right?

Yeah, that's what I thought as well.

@fitzgen
Copy link
Member Author

fitzgen commented Mar 30, 2023

FWIW, I can repro in the wasmtime repository in, for example, the code for wasmtime::Func::new at crates/wasmtime/src/func.rs line 354.

image

@Veykril
Copy link
Member

Veykril commented Mar 30, 2023

Out of curiosity, do the logs complain about cyclic dependencies? And does r-a also complain about cfg(cranelift) annotated things? (I know that cranelift is a default feature in wasmtime, but it might be that r-a gets confused about that for some other reason which would cause htis problem then).

Will look into this myself tomorrow otherwise

@fitzgen
Copy link
Member Author

fitzgen commented Mar 30, 2023

Can you point me at a resource for how to get r-a logs?

@Veykril
Copy link
Member

Veykril commented Mar 30, 2023

I only know how to get to those in VSCode which I believe you aren't using 😅
image

It's fine though, I'll checkout wasmtime tomorrow or so

@fitzgen
Copy link
Member Author

fitzgen commented Mar 30, 2023

Yeah, I'm using emacs.

@flodiebold
Copy link
Member

On Emacs, there should be a *rust-analyzer::stderr* buffer (though if you're using eglot it might be different).

@fitzgen
Copy link
Member Author

fitzgen commented Mar 30, 2023

I restarted rust-analyzer and I did get this message in the stderr buffer:

[ERROR project_model::workspace] cyclic deps: wasmtime(CrateId(344)) -> wasi_cap_std_sync(CrateId(323)), alternative path: wasi_cap_std_sync(CrateId(323)) -> wasi_common(CrateId(324)) -> wasmtime(CrateId(344))
[ERROR project_model::workspace] cyclic deps: wasmtime_component_macro(CrateId(380)) -> wasmtime(CrateId(344)), alternative path: wasmtime(CrateId(344)) -> wasmtime_component_macro(CrateId(380))
[ERROR project_model::workspace] cyclic deps: wasmtime_wasi(CrateId(412)) -> wasi_cap_std_sync(CrateId(323)), alternative path: wasi_cap_std_sync(CrateId(323)) -> wasi_common(CrateId(324)) -> wasmtime(CrateId(344)) -> wasmtime_wasi(CrateId(412))
[ERROR project_model::workspace] cyclic deps: wasmtime_wasi(CrateId(412)) -> wasi_common(CrateId(324)), alternative path: wasi_common(CrateId(324)) -> wasmtime(CrateId(344)) -> wasmtime_wasi(CrateId(412))
[ERROR project_model::workspace] cyclic deps: wasmtime_wasi(CrateId(412)) -> wasi_tokio(CrateId(327)), alternative path: wasi_tokio(CrateId(327)) -> wasi_cap_std_sync(CrateId(323)) -> wasi_common(CrateId(324)) -> wasmtime(CrateId(344)) -> wasmtime_wasi(CrateId(412))
[ERROR project_model::workspace] cyclic deps: wasmtime_wasi(CrateId(412)) -> wasmtime(CrateId(344)), alternative path: wasmtime(CrateId(344)) -> wasmtime_wasi(CrateId(412))
[ERROR project_model::workspace] cyclic deps: wiggle(CrateId(425)) -> wasmtime(CrateId(344)), alternative path: wasmtime(CrateId(344)) -> wasmtime_wasi(CrateId(412)) -> wiggle(CrateId(425))
[ERROR project_model::workspace] cyclic deps: wiggle_macro(CrateId(444)) -> wiggle(CrateId(425)), alternative path: wiggle(CrateId(425)) -> wiggle_macro(CrateId(444))
[ERROR project_model::workspace] cyclic deps: wiggle_test(CrateId(445)) -> wiggle(CrateId(425)), alternative path: wiggle(CrateId(425)) -> wiggle_test(CrateId(445))

@Veykril
Copy link
Member

Veykril commented Mar 31, 2023

Then this could likely be caused by us not handling cyclic deps yet, that is known to cause some issues at least #14167

@1024bees
Copy link

We're seeing a similar issue described at esp-rs/esp-template#58. We have a cfg enabled in build.rs, like wasmtime, and details of what we're seeing are detailed in the linked issue

I am not seeing the cyclic deps error message that wasmtime seems to be emitting.

Not trying to conflate these problems -- I can create another issue if maintainers think it is pragmatic. Also I can spend some time debugging this if I could get useful state I can dump. Very open to hack on RA to diagnose this, given some direction!

@svix-jplatte
Copy link

Seeing the same issue without cyclic dependencies, possibly relevant that the package in question contains both lib.rs and main.rs? Or any other info needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo cargo related issues C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

5 participants