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

Detect if code is running via rust-analyzer #17766

Open
rzvxa opened this issue Aug 1, 2024 · 17 comments
Open

Detect if code is running via rust-analyzer #17766

rzvxa opened this issue Aug 1, 2024 · 17 comments
Labels
C-support Category: support questions

Comments

@rzvxa
Copy link

rzvxa commented Aug 1, 2024

Is there any standard way to detect if the code is running with the rust analyzer or not?

I'd like to be able to compile parts of my proc macro only in actual builds and not when the rust analyzer is running it.

@rzvxa rzvxa added the C-support Category: support questions label Aug 1, 2024
@alibektas
Copy link
Member

alibektas commented Aug 1, 2024

How about adding a feature and not including it in rust-analyzer.runnables.extraArgs ?

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

@alibektas First off thanks for your quick response.

I've also found rust-analyzer.cargo.features; This would be an amazing solution if there is a way to enforce it to downstream users of our crate. However, I suppose it should be configured through the LSP client which is a local configuration.

It would've been awesome if we could get a rust_analyzer feature flag to configure our code based on this information, But we can't rely on local configurations and we need this to propagate correctly when our library is used as a dependency in another crate.

I'd appreciate any suggestions to help us achieve such a thing.

I'm up for contributing toward a way to expose a rust_analyzer feature or at the very least environment variable(if a feature is not possible due to how cargo works).

@alibektas
Copy link
Member

alibektas commented Aug 2, 2024

@rzvxa Well there is one way actually. But I don't know if you would like it #17058. But beware : It is still in development. So basically you create a rust-analyzer.toml file near your top most Cargo.toml (omitting rust-analyzer prefix for the related configuration key ofc!). This is the only text-editor independent solution.

Let me know if I can close this issue 👋

And for the next time I would suggest using repo's Discussion for bringing up things like this.

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

Nice it seems like exactly what I want.

And for the next time I would suggest using repo's Discussion for bringing up things like this.

Yes sir, Thanks for the help.

@rzvxa rzvxa closed this as completed Aug 2, 2024
@flodiebold
Copy link
Member

flodiebold commented Aug 2, 2024

I don't think rust-analyzer.toml will have any effect when the library is being used as a dependency? At least that would be very surprising to me. (At least for compilation-related settings, which will be determined by the top-level cargo metadata invocation.)

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

@flodiebold So do you have any suggestion to achieve this across crates? I also had some difficulties configuring the ratoml file; I couldn't find the reason in the source, but again I didn't spend too much time reading through the code.

I've asked about it here: #13529 (comment)
I'd appreciate any help.


On another note. If rust-analyzer.toml isn't going to cut it; Shall I reopen this issue?

@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

There is a cfg we set for rust-analyzer's analysis but its only set within the context of r-a (not when r-a invokes cargo) nor do we set it for dependencies, frankly because we don't want people to specialize on rust-analyzer right now.

More importantly though, toggling proc-macro compilation depending on whether r-a invoked cargo or not is a huge redflag as anyone using your crate would effectively get constant recompilations whenever they alternative between invoking cargo with and without rust-analyzer.

@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

(gonna re-open as a discussion)

@Veykril Veykril reopened this Aug 2, 2024
@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

Thanks for reopening this.

frankly because we don't want people to specialize on rust-analyzer right now.

I get the concerns behind this, But having such a flag would allow a few things that would be impossible otherwise.

  • Skip some stuff to speed up the rust analyzer in big workspaces
  • Control how and when the rust-analyzer expands macros
  • Attributes/Derives can output explicit documentation to only be used with RA preview/tooltips.

I know it is a double-edged sword to have different behaviors with and without RA but it can unlock some crazy optimizations for people who need it.

For example, the use case we have for it is this:

We want our types to have #[repr(C)] but it causes them to have inefficient abi layouts. That's why we have created a lightweight proc-macro that would reorder fields to fix this issue. It allows us to have fully deterministic types with a well-optimized layout. It was going great until we encountered this issue with RA.

image

Sadly for us, RA uses the version after attribute expansion and it breaks our documentation/preview through it. That's why we want to find a way around it by outputting the unoptimized version for the rust analyzer(which has the field ordering as written in the source code) while doing this optimization in actual builds.

I hope you can see the value of it.

More importantly though, toggling proc-macro compilation depending on whether r-a invoked cargo or not is a huge redflag as anyone using your crate would effectively get constant recompilations whenever they alternative between invoking cargo with and without rust-analyzer.

You are right; Maybe an environment variable would be better to avoid shooting ourselves in the leg. Any way to infer this be it at compile time or runtime would do for us.

@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

I hope you can see the value of it.

I feel conflicted about that, we'd effectively show the user incorrect results then. Notably the layout information is just plain wrong in the hover if we did allow that.

I am certainly interested in giving proc-macro powers to interface with the IDE (to aid in completions and similar things), which would most likely allow this to be done in some capacity as well at some point, though I have yet to sit down and think about that more. Anyways, it won't allow for compilation to differ between plain cargo and r-a invoked cargo, if that ever ends up possible I will consider that a bug due to the recompilation problem.

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

I feel conflicted about that, we'd effectively show the user incorrect results then. Notably the layout information is just plain wrong in the hover if we did allow that.

Yeah, that's one issue that we have to compromise; I was going to combat this by explicitly adding a doc comment in RA runs to each type/field stating their actual layout data.

I was thinking about this flag as an unsafe tool that the implementor has to deal with based on their usage. Even if you omit some useless code for optimization in the RA pass you may cause confusion, But that can be sorted out with a lint rule to warn about it.


Is it possible to build the version for RA in a separate directory? What happens if we set the target dir to targets/rust_analyzer? With that can't we have a feature flag that wouldn't cause recompilations?

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

Another way to fix our specific issue is to have some sort of hint attribute that would allow RA to display a pre-expanded version while using the post-expansion version for inference.


Edit:

Having the ratoml work across crates would allow us to implement this without any change to RA internals. So may I ask for technical limitations behind it? Is there any way to achieve this?

@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

Right a separate target dir would circumvent the issue, but that means your crate now sets that as a requirement for the users which is subpar.

@rzvxa
Copy link
Author

rzvxa commented Aug 2, 2024

but that means your crate now sets that as a requirement for the users which is subpar.

True. I guess even if this is off the table we could still use an environment variable to signal this. It should be pretty easy to implement and wouldn't cause trouble for the builds.

@rzvxa
Copy link
Author

rzvxa commented Aug 4, 2024

For now, we have decided to only run our proc macro in release builds and make all debug builds unoptimized to fix the RA issue we've encountered. This allows us to continue development while this discussion is in progress.

With that said we'd appreciate any standard way of detecting if the code is running via RA or not.

@flodiebold
Copy link
Member

Having the ratoml work across crates would allow us to implement this without any change to RA internals. So may I ask for technical limitations behind it? Is there any way to achieve this?

rust-analyzer.toml is for when you're editing the code in question, it's a replacement for IDE configuration; it would make no sense for it to affect the crate when used as a dependency from crates.io. Technically, features of dependencies are determined by Cargo, and rust-analyzer can only influence the build from the top level (i.e. whatever you could do with parameters passed to cargo check).

@rzvxa
Copy link
Author

rzvxa commented Aug 4, 2024

Having the ratoml work across crates would allow us to implement this without any change to RA internals. So may I ask for technical limitations behind it? Is there any way to achieve this?

rust-analyzer.toml is for when you're editing the code in question, it's a replacement for IDE configuration; it would make no sense for it to affect the crate when used as a dependency from crates.io. Technically, features of dependencies are determined by Cargo, and rust-analyzer can only influence the build from the top level (i.e. whatever you could do with parameters passed to cargo check).

You are right, I think both rust-analyzer.toml and exposing the rust_analyzer feature are out of the question. I'm mostly interested in exposing an environment variable now. It would cause the least amount of havoc, doesn't cause recompilations, and is a good way to let people have some limited degree of freedom to debug/extend how RA interacts with their proc macros - or their code - without relying too much on it.

By default you can't have 2 different behaviors in your code because of the lack of a feature flag, In case somebody jumps through all the hoops to allow conditional proc macros it is on them, That should be treated as untested waters and reliability/soundness of the generated results is on the user, Much like when you are writing unsafe code.

We can communicate this by naming the environment variable something like this RUST_ANALYZER_UNSAFE or keep it experimental forever RUST_ANALYZER_EXPERIMENTAL. One would hope it'll deter people from using it carelessly.

With an environment variable, it should be possible to output additional information when a proc-macro is running via RA.

Edit:

I'm not sure how it would work between compilations. Maybe I was wrong and it is only possible through the rust_analyzer feature since we need to recompile between builds.
I don't know if RA is expanding macros on every run or only uses cached data from previous builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-support Category: support questions
Projects
None yet
Development

No branches or pull requests

4 participants