Explore the possibility of rebuilding the core logic using save_analysis

[ibabushkin]

Since librustc_save_analysis is actually a somewhat stable API, a lot of breakage could be avoided (and at the same time, some issues with the checks could be easily addressed).

However, since the API operates essentially on an AST level, a number of issues has to be solved:

• item correspondence has to be reestablished on an AST level, which is less than ideal (and might be even bad for performance).
• no existing type check machinery can be reused, we would fall back to completely reimplementing our own reasoning about types and lifetimes on an API level. At the same time, technical issues would become relatively straightforward to solve.

To summarize, I believe such a change will cause a shift from relatively complex, but also somewhat compact functionality to much more code which will become possibly a bit simpler and tailored to our domain.

cc @Manishearth @gnzlbg you guys might be interested. While this could solve a number of problems on the backend side, we'd be reimplementing a lot of functionality that is already there.

[gnzlbg]

no existing type check machinery can be reused,

The issue is that the library breaks every now and then on nightly, and that makes it unreliable in practice. Both clippy and rustfmt suffer from this problem, but these tools have minimized it to the point that it is no longer an issue:

• when a PR in rustc is merged that breaks these tools, these tools get notified, and are able to implement a fix before the next nightly hits
• even when the next nightly breaks, this is typically resolved on the same day.

I can think about two approaches to this problem:

• do what rustfmt and clippy do, set the library as a tool, and make sure things are fixed quickly when they break
• leave rust-semverver and cargo-semverver here, and move the library component into the rust tree (e.g. in rust-lang/rust/src/libsemverver or similar) and expose the crate with the compiler - this ensures that the library always compiles (maybe moving some pieces from the library into other components as time goes by).

Moving to save_analysis, losing all type information, and having to re-implement all of that here seems like too much work for too little win.

[Manishearth]

I will say that making this a shipped rustup tool requires a lot more buy-in from everyone, and my gut feeling is that this tool is far from ready for that. Hell, it took clippy and RLS forever to do this, and clippy was in a much more mature state when we first started talking about this.

However if you just want nightly breakage notifications you can do what servo does: set a travis cron job for building on nightly. (I'm working on getting access to this org so that I can fix travis)

Also fwiw there are crates that RLS uses that let you slurp save-analysis info and get nicely typed data, if you need.

[ibabushkin]

On 2019-03-04, Manish Goregaokar wrote: I will say that making this a shipped rustup tool requires a lot more buy-in from everyone, and my gut feeling is that this tool is far from ready for that. Hell, it took clippy and RLS forever to do this, and clippy was in a much more mature state when we first started talking about this. However if you just want nightly breakage notifications you can do what servo does: set a travis cron job for building on nightly. (I'm working on getting access to this org so that I can fix travis) Also fwiw there are crates that RLS uses that let you slurp save-analysis info and get nicely typed data, if you need. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #94 (comment)

Yes, having a travis cronjob worked a treat when the repo was still in the nursery, but I was not able to set up CI since the migration. I agree with the sentiment of little gain for a lot of work though, but exploring this course of action was worthwhile, given your suggestion in the dev tools team thread.

[Manishearth]

Yeah to be clear I'm not super familiar with the implementation, however unlike clippy you aren't going to keep accumulating lots of complicated logic so it should be possible to abstract it away in a way that gets you stability, either by using save-analysis or by upstreaming the abstractions.

[eddyb]

Given all the drawbacks of its current design (cc @Xanewok), I strongly disagree with building anything new on top of librustc_save_analysis - I'd much rather integrate the "compare" part of this project into rustc, possibly as its own crate (we do something similar for miri).

EDIT: just saw that @gnzlbg already proposed this upstreaming, so +1 to that!

[Xanewok]

I strongly agree; I think upstreaming the compare bit is the right thing to do rather than trying to reimplement it using save_analysis.