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

Tracking issue for -Zsave-analysis #43606

Closed
nrc opened this issue Aug 2, 2017 · 20 comments · Fixed by #101841
Closed

Tracking issue for -Zsave-analysis #43606

nrc opened this issue Aug 2, 2017 · 20 comments · Fixed by #101841
Labels
A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-design-concerns Status: There are blocking design concerns. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Aug 2, 2017

-Z flags are now unstable. However, we plan to use save-analysis info for the RLS and Rustdoc and we want these to work on stable compilers. Therefore, we need a stable flag. Options:

  • --save-analysis
  • --emit analysis or analysis-info or analysis-data or whatever
  • Keep the -Z flag and give tools a loophole to use it on stable

I propose that the existence of the flag is stable, but that we make no guarantees about the output when using the flag. We might consider guaranteeing that the output is valid JSON (but that includes {}). We could promise that the shape of the ouput won't change, but I'd rather not.

@nrc nrc added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. B-unstable Blocker: Implemented in the nightly compiler and unstable. I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels Aug 2, 2017
@nrc nrc changed the title Stabilise -Zsave-analysis Stabilise -Zsave-analysis Aug 2, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Aug 3, 2017
@nrc
Copy link
Member Author

nrc commented Aug 3, 2017

Another alternative is that the RLS and Rustdoc could use a custom compiler driver. This would mean we would not need a stable CL flag. This seems fine for the RLS (it would mean a marginally more complex build, but nothing too serious) but would mean some extra work for Rustdoc and mean it would only work if distributed with Rustdoc.

(Motivation for avoiding a stable flag is to prevent other users using the save-analysis data and it becoming de facto stable).

@steveklabnik
Copy link
Member

What does "custom compiler driver" mean exactly?

@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2017

It means that you are using rustc as a library. So instead of extending rustc via plugins, you are writing your own rustc binary (but you are reusing all of the boilerplate offered by the rustc_driver that rustc itself uses). You basically end up implementing a small trait, forwarding all methods to rustc's default implementation, and injecting your own code at the appropriate locations.

@nrc
Copy link
Member Author

nrc commented Aug 8, 2017

I've implemented a shim at https://github.com/nrc/rls-rustc I have a couple more features to add, but this should be a drop-in replacement for rustc for the RLS and rustdoc. I'll leave this issue open until we confirm this approach works, but I think we don't need to stabilise -Zsave-analysis any more.

@michaelwoerister michaelwoerister removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels Aug 10, 2017
@staktrace
Copy link
Contributor

@nrc: can you explain how to go about using this wrapper? In particular I'm looking to do a mozilla-central build on taskcluster with save-analysis turned on. Right now I can do this easily by using a rust nightly toolchain and export RUSTFLAGS="-Zsave-analysis". It's not clear to me how I would plug in this wrapper though. (FWIW I would still prefer stabilizing the save-analysis option into rust stable)

@nrc
Copy link
Member Author

nrc commented Jan 29, 2018

You should just be able to make an executable which looks like:

extern crate rls_rustc;

fn main() {
    rls_rustc::run();
}

And then run that executable as a drop-in replacement for rustc.

@nrc
Copy link
Member Author

nrc commented Jan 29, 2018

@staktrace ^

@staktrace
Copy link
Contributor

@nrc Thanks. I think hooking that into taskcluster is probably nontrivial (at least for me) as I would have to create a custom toolchain and override the compiler being used. I think for now I'll continue using the nightly rustc since that's easier and I have it working, but I would still really like having the -Zsave-analysis option in rust stable.

@ishitatsuyuki
Copy link
Contributor

I think we should not make any guarantee of the analysis format, however it worths to expose a lib like rls-analysis bundled with the compiler distribution. It would start with a small subset of types, expanding as we decide, similar to what proc-macro has been.

Stabilising this allows us to have documentation generators other than rustdoc, for example.

@jyn514
Copy link
Member

jyn514 commented Nov 4, 2020

AFAIK rustdoc doesn't and never has used -Z save-analysis, so I'm going to remove the rustdoc label.

@jyn514 jyn514 added A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. and removed A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 4, 2020
@steveklabnik
Copy link
Member

There was serious effort to make it understand it at one point, but that idea was abandoned long ago, so 👍

@wesleywiser
Copy link
Member

Reviewed as part of the compiler team's "backlog bonanza" in the weekly compiler team triage meeting. Given the in-motion plans for Rust Analyzer to officially replace RLS as our IDE experience which does not use -Zsave-analysis, it's not clear that we should still be working to stabilize this feature.

In general, meeting attendees thought we should deprecate and remove this unstable feature instead once RLS is no longer in use. Therefore, I'm marking this S-tracking-design-concerns until we decide to either move forward with stabilization or commit to removing the feature.

@wesleywiser wesleywiser added the S-tracking-design-concerns Status: There are blocking design concerns. label Apr 28, 2022
@jackh726 jackh726 removed the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Apr 28, 2022
@jackh726 jackh726 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 28, 2022
@jackh726 jackh726 changed the title Stabilise -Zsave-analysis Tracking issue for -Zsave-analysis Apr 28, 2022
@ehuss ehuss mentioned this issue Aug 28, 2022
@eddyb
Copy link
Member

eddyb commented Aug 28, 2022

The future of save-analysis was discussed in #100863 (comment) (i.e. on the PR to sunset RLS).

I had some suggestions but @bjorn3 pointed out (in #100863 (comment)) that:

Rust-analyzer has support for emitting LSIF formatted analysis results as well as Sourcegraph's SCIP. This might be a viable alternative for the save-analysis results for most users.

Based on that, I was able to find existing mentions of LSIF regarding searchfox's indexer (the only project I've heard of that still relies on save-analysis, and is used at scale, at the very least inside Mozilla):

Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1761287 I'm guessing the thing to do would be to coordinate with @asutherland and @emilio, and eventually remove save-analysis in favor of RA+LSIF.

(I'm not expecting to be involved, but I wanted to point this out because it's a simpler solution that supersedes the one I described in #100863 (comment))

@asutherland
Copy link

Thanks for reaching out! I noticed the rls removal earlier today and was wondering about the implications on save-analysis!

Yes, searchfox (https://github.com/mozsearch/mozsearch) will ideally move to RA+SCIP or RA+LSIF in the near-to-medium term. We have a meeting tentatively planned for this Friday (2022-09-02) to discuss various searchfox meta issues like this, so I can:

  1. Pass on any planned timelines for save-analysis removal from this issue at the meeting.
  2. Provide more clarity over searchfox's plans after the meeting occurs (which obviously could be delayed), recognizing that while it's great if we can time things so there's no gap in searchfox's rust indexing support, searchfox's (lack of) resourcing should not impede rust's removal of an unstable feature that has a clear migration path.

@est31
Copy link
Member

est31 commented Aug 29, 2022

Does rust-analyzer support a bulk mode yet? Or is it still request based? I need the usage graph for all items in a given crate, so that I can determine which items are unused. (edit: this is for warnalyzer)

@bjorn3
Copy link
Member

bjorn3 commented Aug 29, 2022

You can use the LSIF or SCIP output formats of rust-analyzer for that I think.

@Veykril
Copy link
Member

Veykril commented Aug 29, 2022

r-a doesn't have a simple way to just fetch all item declarations of a crate, but you can query declarations of a module, so you should be able to collect all item declarations of a crate by traversing the module hierarchy (this does leave out local items though I believe, not sure if those are easily accessible without resolving from the file source directly).

If with request based you mean language server protocol requests, then no, r-a can be used as a library without going through the request protocol.

@eddyb
Copy link
Member

eddyb commented Aug 31, 2022

@est31 @Veykril I believe @bjorn3 is talking about rust-analyzer lsif path/to/crate/dir which looks to be working for me? Or am I missing some nuance here?

I assume worst case you can use the LSIF as a rough skeleton and then query more specific things based on it, but I'm not exactly sure.

@asutherland
Copy link

As a follow-up for searchfox (https://github.com/mozsearch/mozsearch) plans:

  • https://bugzilla.mozilla.org/show_bug.cgi?id=1761287 continues to be the bug we will track any work on the transition to RS+SCIP or RA+LSIF
  • Currently the expectation is that @emilio will end up fixing this reactively when save-analysis is removed. (Although it's possible we could end up with some clangd synergies in which case we can try and address this earlier. Emilio may also be able to find some time earlier as well.) As such, I wouldn't block removal on searchfox.

@nnethercote
Copy link
Contributor

I have created #101841 which removes save analysis, just to see how hard it was: not particularly hard.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2023
…-Simulacrum

Remove save-analysis.

Most tests involving save-analysis were removed, but I kept a few where the `-Zsave-analysis` was an add-on to the main thing being tested, rather than the main thing being tested.

Closes rust-lang#43606
@bors bors closed this as completed in 22a5125 Feb 16, 2023
facebook-github-bot pushed a commit to facebookincubator/Glean that referenced this issue May 3, 2023
Summary:
The flag itself no longer exists in rustc and this indexer never quite worked for our use cases.
We will use rust-analyzer instead, which has SCIP and LSIF support already.

-  D45292993 removed the build flavor we relied on
- see  rust-lang/rust#43606 for removal

Reviewed By: simonmar

Differential Revision: D45516746

fbshipit-source-id: 684117415c5daebca7656c330592f1362781ab1c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-design-concerns Status: There are blocking design concerns. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.