No auto completion on 7-20 nightly version

[w93163red]

Ra Version:

INFO [7/21/2020, 11:12:12 AM]: Extension version: 0.3.249-nightly
INFO [7/21/2020, 11:12:12 AM]: Using configuration {
  diagnostics: { enable: true, warningsAsInfo: [], warningsAsHint: [] },
  lruCapacity: null,
  files: { watcher: 'client', exclude: [] },
  notifications: { cargoTomlNotFound: true },
  cargo: {
    autoreload: true,
    noDefaultFeatures: false,
    allFeatures: false,
    features: [],
    loadOutDirsFromCheck: false,
    target: null
  },
  rustfmt: { extraArgs: [], overrideCommand: null },
  checkOnSave: {
    enable: true,
    extraArgs: [],
    command: '',
    overrideCommand: null,
    allTargets: false,
    allFeatures: null,
    features: null
  },
  cargoRunner: null,
  runnableEnv: null,
  inlayHints: {
    enable: true,
    typeHints: true,
    chainingHints: true,
    parameterHints: true,
    maxLength: 40
  },
  completion: {
    addCallParenthesis: true,
    addCallArgumentSnippets: true,
    postfix: { enable: true }
  },
  callInfo: { full: true },
  updates: { channel: 'nightly', askBeforeDownload: true },
  serverPath: null,
  trace: { server: 'off', extension: false },
  procMacro: { enable: false },
  debug: {
    engine: 'vadimcn.vscode-lldb',
    sourceFileMap: {
      '/rustc/<id>': '${env:USERPROFILE}/.rustup/toolchains/<toolchain-id>/lib/rustlib/src/rust'
    },
    openDebugPane: true,
    engineSettings: {}
  },
  lens: { enable: true, run: true, debug: true, implementations: true },
  hoverActions: {
    enable: true,
    implementations: true,
    run: true,
    debug: true,
    gotoTypeDef: true
  },
  linkedProjects: null,
  withSysroot: true,
  'cargo-watch': { command: 'clippy' }
}
INFO [7/21/2020, 11:12:12 AM]: PersistentState: {
  lastCheck: 1595343463651,
  releaseId: 28761904,
  serverVersion: '0.3.249-nightly'
}
INFO [7/21/2020, 11:12:12 AM]: Using server binary at /Users/xxx/.vscode-server/data/User/globalStorage/matklad.rust-analyzer/rust-analyzer-mac

Ra Log:

Verbose log: https://gist.github.com/w93163red/fb1709102d88916ae4ce0ff0337cf5da

[matklad]

Note that the issue is unactionable (impossible to fix) given the provided information.

See https://rust-analyzer.github.io/manual.html#troubleshooting for how to debug such problems.

[w93163red]

@matklad Sorry for the unclear info! I have updated the info accordingly.

[Emilgardis]

I think I found the commit that introduced the same issue for me at least. with 980a67f44629ed67a54b603aaf9d015a81d61f7a completions seemingly work in every file in my workspace, with 69b79e3a73f9a1b820cf6d5ebc9968d8b08d4e68 completion breaks in at least one file.

I suspect https://github.com/rust-analyzer/rust-analyzer/blob/69b79e3a73f9a1b820cf6d5ebc9968d8b08d4e68/crates/vfs/src/file_set.rs#L72-L82 is the cause of this, taking a long time for some things, not sure how to prove it though. Guidance for troubleshooting would be happily accepted :)

Info

ra client ``` INFO [7/25/2020, 3:03:44 AM]: Extension version: 0.4.0-dev INFO [7/25/2020, 3:03:44 AM]: Using configuration { diagnostics: { enable: true, warningsAsInfo: [], warningsAsHint: [] }, lruCapacity: 30, files: { watcher: 'client', exclude: [] }, notifications: { cargoTomlNotFound: true }, cargo: { noDefaultFeatures: false, allFeatures: false, features: [], loadOutDirsFromCheck: true, target: null }, rustfmt: { extraArgs: [], overrideCommand: null }, checkOnSave: { enable: true, extraArgs: [], command: 'clippy', overrideCommand: null, allTargets: false, allFeatures: null, features: null }, cargoRunner: null, runnableEnv: null, inlayHints: { enable: true, typeHints: true, chainingHints: false, parameterHints: true, maxLength: 20 }, completion: { addCallParenthesis: true, addCallArgumentSnippets: true, postfix: { enable: true } }, callInfo: { full: true }, updates: { channel: 'nightly', askBeforeDownload: true }, serverPath: 'C:/Users/Emil/.cargo/bin/rust-analyzer.exe', trace: { server: 'verbose', extension: true }, procMacro: { enable: true }, debug: { engine: 'auto', sourceFileMap: { '/rustc/': '${env:USERPROFILE}/.rustup/toolchains//lib/rustlib/src/rust' }, openDebugPane: false, engineSettings: {} }, lens: { enable: false, run: true, debug: true, implementations: true }, hoverActions: { enable: true, implementations: true, run: true, debug: true, gotoTypeDef: true }, linkedProjects: null, withSysroot: true } INFO [7/25/2020, 3:03:44 AM]: PersistentState: { lastCheck: 1592251425728, releaseId: 27538406, serverVersion: '0.3.187-nightly' } INFO [7/25/2020, 3:03:44 AM]: Using server binary at C:/Users/Emil/.cargo/bin/rust-analyzer.exe DEBUG [7/25/2020, 3:03:44 AM]: Checking availability of a binary at C:/Users/Emil/.cargo/bin/rust-analyzer.exe DEBUG [7/25/2020, 3:03:44 AM]: C:/Users/Emil/.cargo/bin/rust-analyzer.exe --version: { status: 0, signal: null, output: [ null, 'rust-analyzer 69b79e3\n', '' ], pid: 101612, stdout: 'rust-analyzer 69b79e3\n', stderr: '' } ``` RA status after triggering completion on something in the file which can't complete with 69b79e3a73f9a1b820cf6d5ebc9968d8b08d4e68 and waiting until results show, which only include non-ra results ``` workspaces: 304 packages loaded analysis: 6664 (66mb) files 0 (0b) symbols 2826 trees, 30 retained 46611 trees, 30 retained (macros) memory: 0b allocated 0b resident gc 51 seconds ago requests: * 54 textDocument/codeAction 14445ms 50 textDocument/codeAction 15572ms 66 textDocument/codeAction 11038ms 53 textDocument/codeAction 15045ms 63 textDocument/codeAction 11445ms 49 textDocument/codeAction 16170ms 67 textDocument/codeAction 10440ms 62 textDocument/codeAction 12045ms 70 textDocument/codeAction 10039ms 88 textDocument/codeAction 5ms ``` `nightly-x86_64-pc-windows-msvc rustc 1.46.0-nightly (346aec9b0 2020-07-11)`

[lnicola]

@Emilgardis interesting, see also https://github.com/rust-analyzer/rust-analyzer/pull/5252#issuecomment-656065453 which is older than https://github.com/rust-analyzer/rust-analyzer/commit/980a67f44629ed67a54b603aaf9d015a81d61f7a but related to https://github.com/rust-analyzer/rust-analyzer/commit/69b79e3a73f9a1b820cf6d5ebc9968d8b08d4e68.

@edwin0cheng do you still have historical data? I should have pasted a screenshot in my comment above.

[Emilgardis]

After doing some more debugging, I'm starting to think the old implementation satisfied a condition not satisfied by the new implementation.

I reverted file_set.rs only to the "bad-trie" implementation (which had completions working) , and did some eprintlns to check latency, but timing was pretty much the same. One difference though is the output, still checking on it but it does seem reverting it fixes the issue.

[Emilgardis]

Changing FileSetConfig::partition to

pub fn partition(&self, vfs: &Vfs) -> Vec<FileSet> {
        let mut scratch_space = Vec::new();
        let mut res = vec![FileSet::default(); self.len()];
        eprintln!("Started partition:");
        for (file_id, path) in vfs.iter() {
            let root = self.classify(&path, &mut scratch_space);
            eprintln!("\tidx = {} for {:?}", root, path);
            res[root].insert(file_id, path.clone())
        }
        res
    }

and doing the same to the "bad-trie" impl reveals the following difference (Note I've tried to anonymize the contents and removed noise)

So, crate-b\\src\\context\\mod.rs (which is the file I'm trying to autocomplete in), gets misclassified as crate-a.

crate-b is named foo-things, and crate-a is named foo, if that is of any significance.

EDIT: Some more investigating, seems this error is only present on this file when the vs workspace loads up with it open in an editor. Closing the file in editor, making sure there's another workspace rs file in editor tabs, saving workspace layout (by closing vsc) and reopening workspace makes the file able to complete after opening it again.

[matklad]

So, the trace from original PR looks like the completion is legitimately cancelled by typing more stuff, and it seems like the client hadn't re-sent the request for some reason:

[Trace - 11:35:59 AM] Sending request 'textDocument/completion - (163)'.
[Trace - 11:35:59 AM] Sending notification 'textDocument/didChange'. <- this'll invalidate completions from the previous requests
[Trace - 11:35:59 AM] Received response 'textDocument/completion - (163)' in 93ms. Request failed: content modified (-32801). <- so that's why we cancel them

Although, produced no valid metatdata sounds like there might be some project structure problems?

[matklad]

About @Emilgardis issue, not sure what's going on here. I've added the test for prefixed paths: https://github.com/rust-analyzer/rust-analyzer/pull/5537.

This is windows, so it could be that we don't normilize paths in the same way somewhere, especially for in-memory documents.

For both problems (which seem like they are different) having a minimal reproducible example would help.

[edwin0cheng]

@lnicola :

[lnicola]

Oh, I forgot about the pagination links :facepalm:.

[theli-ua]

I'm not sure if related but I seems to have lost any analysis after the aforementioned commit as well. For example on this

use std::collections::XXX;
fn main() {
    println!("Hello, world!");
}

won't give me diagnostics that use is incorrect. while it works perfectly fine when built from 980a67f works perfectly fine. However, this happens only if I set rust-analyzer.procMacro.enable to true.

[lnicola]

@theli-ua see https://github.com/rust-analyzer/rust-analyzer/pull/5651 if you're using nightly.

[pragmatrix]

@Emilgardis @matklad

This is windows, so it could be that we don't normalize paths in the same way somewhere

I think I've found a problem with the FileSet partitioning: It seems that paths on Windows are entering RA with either uppercase or lowercase drive letters, not a problem, because Path, AbsPath, VfsPath all handle that correctly and treat the drive letter case insensitive, but not so the VfsPath::encode function that is used as a basis for partitioning the file sets: As soon the encode function returns consistent results in regard to the drive letter casing, lots of Windows problems disappear, for example the module not found problem described in #5484. Here is a hack f0d511e6d0f65a1c6ee40b7ea3a2b57d76863022 that I am testing right now.

[Emilgardis]

Indeed, I discovered this (but not that it mattered) as well but never got to implement it or report it as the error I was having seems to be gone for me now either way.

I implemented a fix for this stealing code from the path normalization in the ra path workspace crate, unfortunately though we need a unstable fearure to make the drive letter uppercase.

[matklad]

Excellent catch @pragmatrix ! Could you check if the following works for you?

    // Don't make this `pub`
    pub(crate) fn encode(&self, buf: &mut Vec<u8>) {
        let tag = match &self.0 {
            VfsPathRepr::PathBuf(_) => 0,
            VfsPathRepr::VirtualPath(_) => 1,
        };
        buf.push(tag);
        match &self.0 {
            VfsPathRepr::PathBuf(path) => {
                #[cfg(windows)]
                {
                    use std::os::windows::ffi::OsStrExt;
                    for component in path.components() {
                        buf.push(b'\\');
                        for wchar in component.as_os_str().encode_wide() {
                            buf.extend(wchar.to_le_bytes().iter());
                        }
                    }
                }
                #[cfg(unix)]
                {
                    use std::os::unix::ffi::OsStrExt;
                    let path: &std::ffi::OsStr = it.as_os_str();
                    buf.extend(path.as_bytes());
                }
                #[cfg(not(any(windows, unix)))]
                {
                    let path: &std::ffi::OsStr = it.as_os_str();
                    buf.extend(path.to_string_lossy().as_bytes());
                }
            }
            VfsPathRepr::VirtualPath(VirtualPath(s)) => buf.extend(s.as_bytes()),
        }
    }

I think components should do normalization here: https://github.com/rust-lang/rust/blob/cee14d8c300feb2ac2a949848911bf4c3b143e70/library/std/src/sys/windows/path.rs#L61

[Emilgardis]

VerbatimDisk is uncommon, it should also be https://github.com/rust-lang/rust/blob/cee14d8c300feb2ac2a949848911bf4c3b143e70/library/std/src/sys/windows/path.rs#L86

[pragmatrix]

I don't know why, but using the path components does not seem to change the drive letter, the same symptoms reappeared, here is a minimal test-case:

#[cfg(windows)]
#[test]
fn test_encode_consistency_on_windows() {
    let p1 = VfsPath::from(AbsPathBuf::try_from("C:\\t").unwrap());
    let p2 = VfsPath::from(AbsPathBuf::try_from("c:\\t").unwrap());

    let mut buf1 = Vec::new();
    let mut buf2 = Vec::new();
    p1.encode(&mut buf1);
    p2.encode(&mut buf2);
    assert_eq!(buf1, buf2);
}

which fails:

thread 'vfs_path::test_encode_consistency_on_windows' panicked at 'assertion failed: `(left == right)`
  left: `[0, 92, 67, 0, 58, 0, 92, 92, 0, 92, 116, 0]`,
 right: `[0, 92, 99, 0, 58, 0, 92, 92, 0, 92, 116, 0]`', crates\vfs\src\vfs_path.rs:98:5

• 67 -> 'C'
• 99 -> 'c'

[Veykril]

Is this still relevant? From the looks of it, #5756 implemented a fix to the mentioned problem here right?

[Emilgardis]

I have not encountered this specific issue anymore. Other issues have been solved with multiple proc-macro abis and other fixes.

Im sure there are other cases but this specific one is solved imo

[Veykril]

Then let's close this, if it pops up again for anyone it should go into a new issue.