Re-add rust-analyzer as a subtree #99465

fasterthanlime · 2022-07-19T12:47:10Z

(Disclaimer: THIS IS A DRAFT, NOBODY APPROVED THIS, I'M JUST TRYING THINGS OUT)

This PR shows what it would take to make rust-lang/rust-analyzer#12815 happen.

I simply removed the submodule:

$ git rm -f src/tools/rust-analyzer

And added a subtree, after patching my local git-subtree, following the rustc dev guide:

$ git subtree add -P src/tools/rust-analyzer https://github.com/rust-lang/rust-analyzer.git master
(lotsa output + 5K+ commits added)

I then cherry-picked some commits from:

The goal being to avoid regressions to rust-analyzer's proc-macro-srv component by running its test suite in CI.

As a reminder, nobody approved this, this is me just doing research to try and save time if and when that option is chosen.

internal: Lift out IdentContext from CompletionContext Makes things a bit messy overall, but with this I can start cleaning up the render functions properly now. cc rust-lang/rust-analyzer#12571

fix: Don't trigger pattern completions when typing a wildcard pattern Fixes rust-lang/rust-analyzer#12592

Don't analyze dependencies with `test`; this should fix various cases where crates use `cfg(not(test))` and so we didn't find things. "Local" here currently means anything that's not from the registry, so anything inside the workspace, but also path dependencies. So this isn't perfect, and users might still need to use `rust-analyzer.cargo.unsetTest` for these in some cases.

fix: Fix auto-ref completions inserting into wrong locations Fixes rust-lang/rust-analyzer#8058

fix: Only apply `cfg(test)` for local crates Don't analyze dependencies with `test`; this should fix various cases where crates use `cfg(not(test))` and so we didn't find things. "Local" here currently means anything that's not from the registry, so anything inside the workspace, but also path dependencies. So this isn't perfect, and users might still need to use `rust-analyzer.cargo.unsetTest` for these in some cases.

internal: Simplify some completions

- remove Valid, it serves no purpose and just obscures the diff - rename some things - don't use is_valid_candidate when searching for impl, it's not necessary

…eykril fix: completes non exhaustive variant within the defining crate close rust-lang#12624

Fixes: rust-lang#12615

internal: Simplify

jyn514 · 2022-07-19T20:20:12Z

error[E0725]: the feature proc_macro_internals is not in the list of allowed features

this is coming from --check-cfg, you'll need to add those cfgs to the list of known cfgs.

jyn514 · 2022-07-19T20:55:31Z

src/bootstrap/dist.rs

+            .ensure(tool::RustAnalyzer {
+                compiler,
+                target,
+                extra_features: vec!["in-rust-tree".to_owned()]


Rather then adding this at every call site, you could make RustAnalyzer its own step outside of the macro, which calls ensure(ToolBuild) internally with the right features.

I'm now working on the real PR and I'm trying to to understand this comment.

Do you mean adding a separate struct (what would it be named?) that implements Step manually? And the macro-generated Step would call it, like Rls ensures Clippy?

I'm trying to wrap my mind around this.

Ohh do you mean not using the macro at all?

correct, not using the macro at all. or you can change the macro not to have extra_features in some cases only; but that seems more complicated. up to you which you want to do.

jyn514 · 2022-07-19T21:00:47Z

src/bootstrap/check.rs

+    const DEFAULT: bool = true;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.paths(&["src/tools/rust-analyzer", "rust-analyzer"])


That second one is redundant, the builder already has logic that lets you abbreviate to the last component of the path.

Suggested change

run.paths(&["src/tools/rust-analyzer", "rust-analyzer"])

run.paths(&["src/tools/rust-analyzer"])

jyn514 · 2022-07-19T21:01:48Z

src/bootstrap/check.rs

+        let compiler = builder.compiler(builder.top_stage, builder.config.build);
+        let target = self.target;
+
+        builder.ensure(Rustc { target });


This doesn't look right, rust-analyzer doesn't use rustc_private.

I copied that from the tool_check_step! macro (used for clippy, miri, rls, rustfmt, etc.)

Right, those use rustc_private. Rust-analyzer doesn't.

I'm looking at this again and, without it, I get some failures like:

error[E0463]: can't find crate for `core` error[E0463]: can't find crate for `std` error[E0463]: can't find crate for `compiler_builtins` error[E0463]: can't find crate for `std` --> /home/amos/.cargo/registry/src/github.com-1ecc6299db9ec823/text-size-1.1.0/src/traits.rs:1:23 | 1 | use {crate::TextSize, std::convert::TryInto}; | ^^^ can't find crate error: cannot find attribute `derive` in this scope --> /home/amos/.cargo/registry/src/github.com-1ecc6299db9ec823/drop_bomb-0.1.5/src/lib.rs:118:3 | 118 | #[derive(Debug)] | ^^^^^^

I think we need at least ensure(Std {}), maybe even ensure(Rustc {})

Continued here: #99603 (comment)

jyn514 · 2022-07-19T21:02:12Z

src/bootstrap/check.rs

+        let mut cargo = prepare_tool_cargo(
+            builder,
+            compiler,
+            Mode::ToolRustc,


Suggested change

Mode::ToolRustc,

Mode::ToolStd,

This reverts commit c213c23.

rust-log-analyzer · 2022-07-20T11:49:59Z

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking lock_api v0.4.7
    Checking always-assert v0.1.2
    Checking ena v0.14.0
    Checking tracing-log v0.1.3
    Checking sourcegen v0.0.0 (/checkout/src/tools/rust-analyzer/crates/sourcegen)
    Checking flate2 v1.0.24
    Checking xflags v0.2.4
    Checking parking_lot_core v0.9.3
    Checking stdx v0.0.0 (/checkout/src/tools/rust-analyzer/crates/stdx)
---
    Checking crossbeam-deque v0.8.1
    Checking dashmap v5.3.4
    Checking parking_lot v0.12.1
    Checking parking_lot v0.11.2
    Checking xtask v0.1.0 (/checkout/src/tools/rust-analyzer/xtask)
    Checking pulldown-cmark v0.9.1
    Checking regex-automata v0.1.10
    Checking regex v1.5.6
    Checking text-edit v0.0.0 (/checkout/src/tools/rust-analyzer/crates/text-edit)
---

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools/release/build/proc-macro-test-2f04565cab8a716e/build-script-build` (exit status: 101)
  --- stdout
  proc-macro-test failed to build, detailed output follows:
  === nested cargo stdout ===


  === nested cargo stderr ===
  error: the lock file /checkout/src/tools/rust-analyzer/crates/proc-macro-test/imp/Cargo.lock needs to be updated but --locked was passed to prevent this
  If you want to try to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.


  --- stderr
  thread 'main' panicked at 'proc-macro-test failed to build', crates/proc-macro-test/build.rs:36:9

jyn514 · 2022-07-20T15:11:36Z

src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_sysroot/rustc_server.rs

@@ -288,7 +288,7 @@ impl server::FreeFunctions for Rustc {
        &mut self,
        _s: &str,
    ) -> Result<bridge::Literal<Self::Span, Self::Symbol>, ()> {
-        todo!("implement literal_from_str")
+        panic!("FIXME: implement literal_from_str")


this specifically shouldn't need to change I think, only the comments

RA has a tidy check that caught this todo!.

I've switched strategies: I'm going to implement all that in the RA repository first (with cargo +stage1 test --features rust-analyzer/in-tree) - there's been too many bridge changes to just tackle them in this monster PR.

fasterthanlime · 2022-07-20T15:27:26Z

This PR has served its purpose — thanks to @jyn514 for assisting me in doing the research.

The roadmap for "RA as a git subtree" is already well underway:

[Tracking issue] rust-analyzer in-tree build prerequisites rust-analyzer#12818

After these changes land there, I'll re-attempt a subtree PR with The Power Of Hindsight (simpler bootstrap/ code, hopefully).

Veykril and others added 30 commits June 20, 2022 17:41

internal: Lift out IdentContext from CompletionContext

6e9c963

Auto merge of rust-lang#12594 - Veykril:completions, r=Veykril

a1ff3ca

internal: Lift out IdentContext from CompletionContext Makes things a bit messy overall, but with this I can start cleaning up the render functions properly now. cc rust-lang/rust-analyzer#12571

fix: Don't trigger pattern completions when typing a wildcard pattern

1f02840

Auto merge of rust-lang#12596 - Veykril:completions, r=Veykril

9fdfa9f

fix: Don't trigger pattern completions when typing a wildcard pattern Fixes rust-lang/rust-analyzer#12592

fix: Fix auto-ref completions inserting into wrong locations

06ee4d6

Remove pattern rendering hack

46d2271

Refactor a bit

05ca0a2

Reimplement auto-ref completions for fields

8b07898

Auto merge of rust-lang#12597 - Veykril:completions, r=Veykril

439a513

fix: Fix auto-ref completions inserting into wrong locations Fixes rust-lang/rust-analyzer#8058

Fix test

07d78b6

internal: Simplify some completions

7a0774d

Auto merge of rust-lang#12604 - Veykril:completions, r=Veykril

6fc5c3c

internal: Simplify some completions

determine doc link type from start instead of text or code

c6f776c

handle autolink as edge case

b7e3f52

apply suggestions

f780145

add use_trivial_contructor.rs

f9379df

add doc strings to use_trivial_contructor.rs

817082c

condense matches on autolink

96ff235

goto where trait method impl

1ef5e14

reformat code

1064c75

Various cleanups

29f01cd

- remove Valid, it serves no purpose and just obscures the diff - rename some things - don't use is_valid_candidate when searching for impl, it's not necessary

Add two more tests

f410fdf

Fix unstable feature use

36fadc4

fix: completes non exhaustive variant within the defining crate

472ae16

Auto merge of rust-lang#12625 - yue4u:fix/non-exhaustive-variant, r=V…

19fad54

…eykril fix: completes non exhaustive variant within the defining crate close rust-lang#12624

fix: trailing ':' on empty inactive reasons

e0c17e8

Fixes: rust-lang#12615

internal: Simplify

2642f64

Auto merge of rust-lang#12628 - Veykril:simplify, r=Veykril

32b40de

internal: Simplify

Add (and use) sysroot ABI when building rust-analyzer in-tree

b189b49