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

unresolved extern crate for test crate #6714

Closed
FichteFoll opened this issue Dec 3, 2020 · 12 comments · Fixed by #10385
Closed

unresolved extern crate for test crate #6714

FichteFoll opened this issue Dec 3, 2020 · 12 comments · Fixed by #10385
Labels
S-actionable Someone could pick this issue up and work on it right now

Comments

@FichteFoll
Copy link

I'm using benchmarking on nightly through the test feature, which requires extern crate test in the code. However, rust-anaylzer always marks this as unresolved extern crate and I have seen no other reports of this,
so I'm asking here whether that is a bug from rust-analyzer that it doesn't know about this default crate or whether I did a misconfiguration by using nightly.

I am running Arch Linux and the LSP plugin for Sublime Text.

$ rust-analyzer --version
rust-analyzer ???????

$ pacman -Qs rust-analyzer
local/rust-analyzer 20201130-1
    Experimental Rust compiler front-end for IDEs
@bjorn3
Copy link
Member

bjorn3 commented Dec 3, 2020

https://github.com/rust-analyzer/rust-analyzer/blob/10e3a9879c8714320f9a0729d647da7877f0a753/crates/project_model/src/sysroot.rs#L40 only lists core, alloc and std. Maybe test isn't included as it is an unstable crate?

@lnicola lnicola added the S-actionable Someone could pick this issue up and work on it right now label Dec 20, 2020
@doivosevic
Copy link

same. can we please get a response on this?

@flodiebold
Copy link
Member

only lists core, alloc and std. Maybe test isn't included as it is an unstable crate?

These crates are unconditionally added as dependencies to each crate, IIRC. We probably don't want to do that for test. The problem is that we can't deal with the "it's only a dependency if there's an extern crate declaration" logic, since we need a fixed crate graph.

@kawogi
Copy link

kawogi commented Jan 28, 2021

I wonder how/if rust-analyzer handles the no_std case. In this scenario the std entry would be unnecessary.

What would happen if the test crate was added to that list?

@flodiebold
Copy link
Member

flodiebold commented Jan 29, 2021

We don't handle the no_std case. In principle, we should be handling it the same way, yes.

Adding test to that list would mean you can just use test::* without an extern crate declaration, which is not how it works in rustc. It also probably means we'd always analyze the test crate, which would have a performance impact. It would probably not have terrible consequences, but considering the vast majority of crates does not use test, I'm not convinced it would be worth working around in this way before we fix it properly. (You can add unresolved-extern-crate to rust-analyzer.diagnostics.disabled to get rid of the error.)

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 5, 2021

I think we meet the same issue in priroda (cc @bjorn3), where priroda depends on miri, which has extern crate rustc_mir and friends. In this case, our root crate also depends on these crates, which works fine. It would be nice if there was at least some workaround

(Could building a custom crate graph JSON work? It would be nice to have some way to manually specify 'miri' depends on the rustc private crates)

DJMcNab added a commit to DJMcNab/priroda that referenced this issue Mar 5, 2021
@bjorn3
Copy link
Member

bjorn3 commented Mar 5, 2021

For rustc_* there is the rust-analyzer.rustcDev config that you can point to Cargo.toml inside a local rustc checkout.

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 6, 2021

Yes, I do have that set up, and I think it's set up properly.

That works for the root crate (priroda), but it doesn't work for the dependency crate (miri)

bors bot added a commit that referenced this issue Mar 8, 2021
7891: Improve handling of rustc_private r=matklad a=DJMcNab

This PR changes how `rust-analyzer` handles `rustc_private`. In particular, packages now must opt-in to using `rustc_private` in `Cargo.toml`, by adding:
```toml
[package.metadata.rust-analyzer]
rustc_private=true
```

This means that depending on crates which also use `rustc_private` will be significantly improved, since their dependencies on the `rustc_private` crates will be resolved properly.

A similar approach could be used in #6714 to allow annotating that your package uses the `test` crate, although I have not yet handled that in this PR.

Additionally, we now only index the crates which are transitive dependencies of `rustc_driver` in the `rustcSource` directory. This should not cause any change in behaviour when using `rustcSource: "discover"`, as the source used then will only be a partial clone. However, if `rustcSource` pointing at a local checkout of rustc, this should significantly improve the memory usage and lower indexing time. This is because we avoids indexing all crates in `src/tools/`, which includes `rust-analyzer` itself.

Furthermore, we also prefer named dependencies over dependencies from `rustcSource`. This ensures that feature resolution for crates which are depended on by both `rustc` and your crate uses the correct set for analysing your crate.

See also [introductory zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Fixed.20crate.20graphs.20and.20optional.20builtin.20crates/near/229086673)

I have tested this in [priroda](https://github.com/oli-obk/priroda/), and it provides a significant improvement to the development experience (once I give `miri` the required data in `Cargo.toml`)

Todo:
- [ ] Documentation

This is ready to review, and I will add documentation if this would be accepted (or if I get time to do so anyway)

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
bors bot added a commit that referenced this issue Mar 8, 2021
7891: Improve handling of rustc_private r=matklad a=DJMcNab

This PR changes how `rust-analyzer` handles `rustc_private`. In particular, packages now must opt-in to using `rustc_private` in `Cargo.toml`, by adding:
```toml
[package.metadata.rust-analyzer]
rustc_private=true
```

This means that depending on crates which also use `rustc_private` will be significantly improved, since their dependencies on the `rustc_private` crates will be resolved properly.

A similar approach could be used in #6714 to allow annotating that your package uses the `test` crate, although I have not yet handled that in this PR.

Additionally, we now only index the crates which are transitive dependencies of `rustc_driver` in the `rustcSource` directory. This should not cause any change in behaviour when using `rustcSource: "discover"`, as the source used then will only be a partial clone. However, if `rustcSource` pointing at a local checkout of rustc, this should significantly improve the memory usage and lower indexing time. This is because we avoids indexing all crates in `src/tools/`, which includes `rust-analyzer` itself.

Furthermore, we also prefer named dependencies over dependencies from `rustcSource`. This ensures that feature resolution for crates which are depended on by both `rustc` and your crate uses the correct set for analysing your crate.

See also [introductory zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Fixed.20crate.20graphs.20and.20optional.20builtin.20crates/near/229086673)

I have tested this in [priroda](https://github.com/oli-obk/priroda/), and it provides a significant improvement to the development experience (once I give `miri` the required data in `Cargo.toml`)

Todo:
- [ ] Documentation

This is ready to review, and I will add documentation if this would be accepted (or if I get time to do so anyway)

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
DJMcNab added a commit to DJMcNab/priroda that referenced this issue Mar 9, 2021
@pymongo
Copy link
Contributor

pymongo commented Apr 14, 2021

I think we need to add a config like rust-analyzer.rustcSource

"rust-analyzer.rustcSource": "/home/w/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/rustc-src/rust/compiler/rustc_driver/Cargo.toml"

rust-analyzer.rust_test_crate_source config example:

"rust-analyzer.rust_test_crate_source": "/home/w/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/Cargo.toml"

@workingjubilee
Copy link
Member

These crates are unconditionally added as dependencies to each crate, IIRC. We probably don't want to do that for test. The problem is that we can't deal with the "it's only a dependency if there's an extern crate declaration" logic, since we need a fixed crate graph.

@flodiebold This is just my curiosity here but what constraints exactly require a "fixed crate graph"? I know about rust-analyzer's graph-query model in the broad sense, I'm not sure why that requires fixedness here.

@flodiebold
Copy link
Member

What I meant by that is that we need to know the crate graph before analyzing the code, it can't change in response to things we find during name resolution. The reason for that is that the crate graph is an input for rust-analyzer. Of course nothing is impossible, this is just a simplifying assumption, but it's pretty pervasive, I think. I imagine it might be possible to solve this by instead having a "preliminary" crate graph as input, where possible sysroot dependencies are specifically marked, and then having name resolution (the crate_def_map query) also produce the actual dependencies. That would mean that we'd need to expand and name-resolve all crates to get the actual crate graph though, and I'm sure other parts of the code rely on the graph being free to compute, so then we'd have to check whether those parts can/have to use the preliminary graph, and so on.

@Walther
Copy link
Contributor

Walther commented Jul 11, 2021

Polite little bump 🙇‍♂️
Any updates on this - known workarounds, possible approaches for fixing this, other?

Ran into this when using cargo bench, rust-analyzer & vscode.

(You can add unresolved-extern-crate to rust-analyzer.diagnostics.disabled to get rid of the error.)

if i'm not mistaken, this disables all unresolved extern crate warnings, which is probably not an ideal user experience either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants