Skip to content
This repository was archived by the owner on Mar 1, 2019. It is now read-only.

Support multiple crate types and versions (and multiple passed directly in-memory) #93

Closed
Xanewok opened this issue Oct 4, 2017 · 2 comments · Fixed by #106
Closed

Comments

@Xanewok
Copy link
Collaborator

Xanewok commented Oct 4, 2017

Currently master_crate_map maps between crate names (which is the same for lib and bin crate types).
Furthermore, when reloading multiple separate Analysises, I think these are lowered into Ids with bad krate member (always 0, which is LOCAL_CRATE from rustc).

I'll edit this issue when I'll get to something more.

@Xanewok
Copy link
Collaborator Author

Xanewok commented Oct 15, 2017

Okay, so currently the data per each crate is organized in a per_crate map by Option<PathBuf>, which maps directly to respective Analysis data:

  1. From a single None, when the data is passed directly (associated Option<PathBuf> in raw::Crate is later used by the key in the map)
  2. From Some(path), where path leads to a JSON file, from which the data is read

Additionally, when processing/lowering the analysis files, it tries to update a global master_crate_map (String -> u32), per each file reading the local crate and its external dependencies, reading each's name and local crate index. This mapping is later used to transform from crate-local analysis data::Ids to global Ids that can work in the inter-crate context.

This poses a few problems:

  • Right now, in the RLS, we should be able to directly compile and pass directly multiple Analysis from multiple crates, so we need the ability to store them in the per_crate map
  • Technically different crate targets (bin, lib, build script, etc.) are compiled as different crates, but having the same crate name - we need to be able to distinguish them for Id-building (otherwise reloading analysis from bin will overwrite the data from lib, Id-wise)
  • Not only do crates can have different crate targets, a single build can have also multiple versions of the same package lib as an dependency (e.g. one member package pulls in 0.7.1, while other requires 0.8.0, or in case of a single package project different dependencies can also pull multiple versions, that will differ in the API)

While the last concern sounds least practical, I think it's worth taking it into account when designing the solution. I think that the solution boils down to introducing a consistent CrateId key, which could be used as a key to access appropriate PerCrateAnalysis and its globally mapped crate index (for purposes of creating an inter-crate Id).

Additionally, we'd need more information from the compiler in the save-analysis data. I was exploring a bit what could we emit to help identify the crate. Right now I tried to additionally expose external crate source (originating dylib/rlib/rmeta), changes for rustc, rls-data.
Using a simple workspace with a root package depending on 2 workspace member libs, each also depending on incompatible versions of rls-data it gives:

    "prelude": {
        "crate_name": "different_dep_versions",
        "crate_root": "src",
        "external_crates": [
            {
                "file_name": "/home/xanewok/repos/different_dep_versions/src/main.rs",
                "name": "rls_data",
                "num": 17,
                "source": {
                    "dylib": null,
                    "rlib": null,
                    "rmeta": "/home/xanewok/repos/different_dep_versions/target/debug/deps/librls_data-bb60ea1227c10c9b.rmeta"
                }
            },
            ...
            {
                "file_name": "/home/xanewok/repos/different_dep_versions/src/main.rs",
                "name": "rls_data",
                "num": 13,
                "source": {
                    "dylib": null,
                    "rlib": null,
                    "rmeta": "/home/xanewok/repos/different_dep_versions/target/debug/deps/librls_data-e7a3c9e5473a310d.rmeta"
                }
            },
            ...
            {
                "file_name": "/home/xanewok/repos/different_dep_versions/src/main.rs",
                "name": "core",
                "num": 2,
                "source": {
                    "dylib": null,
                    "rlib": "/home/xanewok/repos/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-3181dd9e46400ebd.rlib",
                    "rmeta": null
                }
            },
            {
                "file_name": "/home/xanewok/repos/different_dep_versions/src/main.rs",
                "name": "std",
                "num": 1,
                "source": {
                    "dylib": "/home/xanewok/repos/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-5e8ebc384e5dfd82.so",
                    "rlib": "/home/xanewok/repos/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-5e8ebc384e5dfd82.rlib",
                    "rmeta": null
                }
            },
            ...

It's worth noting that even though in the analysis data for the root package two different rls_datas have different nums, this will not be the case for each member package save-analysis data, where there will be only a single external rls_data, possibly with a same num. This will matter when reading every save-analysis files iteratively, as we can't depend on the crate name and num to make a distinction.

This is meant more to highlight the problem and explore the solution space, rather than propose a concrete one.

@nrc what do you think would be the best course of action here?

@Xanewok
Copy link
Collaborator Author

Xanewok commented Oct 18, 2017

I tried to do some more research, @nikomatsakis said that a crate_name + crate_disambiguator should be a globally unique identifier for a crate, in the context of some world, accounting for things like different crate target or versions, and as such, looks like a perfect candidate for this use case.

I'm currently working on a version using the aforementioned disambiguator instead of using CrateSource and I'm very hopeful about it, as it looks promising! I'll post an update (and possibly PRs) when I'm done.

@Xanewok Xanewok changed the title Improve handling multiple crate types and loading analysis data for multiple 'local' (active?) crates Support multiple crate types and versions (and multiple passed directly in-memory) Oct 23, 2017
bors added a commit to rust-lang/rust that referenced this issue Nov 2, 2017
Emit crate disambiguators in save-analysis data

Needed for rust-dev-tools/rls-analysis#93.
Blocked by rust-dev-tools/rls-data#11. (For now, this pulls my branch [rls-data/crate-source](https://github.com/Xanewok/rls-data/tree/crate-source))

This will allow to disambiguate different crates types/versions when indexing resulting save-analysis data (most importantly allow to support bin+lib and different crate versions).

r? @nrc
@nrc nrc closed this as completed in #106 Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
1 participant