Skip to content

Possible synchronisation issue causes nonsense syntax errors #6402

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

Closed
Timmmm opened this issue Oct 29, 2020 · 10 comments
Closed

Possible synchronisation issue causes nonsense syntax errors #6402

Timmmm opened this issue Oct 29, 2020 · 10 comments
Labels
S-unactionable Issue requires feedback, design decisions or is blocked on other work

Comments

@Timmmm
Copy link
Contributor

Timmmm commented Oct 29, 2020

I unfortunately don't have a way to reproduce this, but within the last couple of weeks I have noticed a new issue - sometimes when I make edits and save the file to trigger cargo check it gives me nonsensical syntax errors. This didn't used to happen. Editing a bit more and saving the file does not help. Reloading the window does.

It feels a lot like Rust-analyzer's internal representation of the code is getting out of sync with VSCode's representation. I have written a couple of language servers and I know the incremental text changing bit is hard to get right - maybe an obscure bug there.

Is there any way to get rust-analyzer to dump what it thinks the current state of a file is?

@kjeremy
Copy link
Contributor

kjeremy commented Oct 29, 2020

Can you attach a screen shot of the problems pane next time this happens? It would be useful to know if these are cargo check diagnostics or RA diagnostics.

@lnicola
Copy link
Member

lnicola commented Oct 29, 2020

Are you sure you're getting cargo check errors? rust-analyzer runs it as if you would run it from the terminal, so it shouldn't see any invalid RA state because there's no way for us to pass it. But we do have some diagnostics, and it's quite possible that these show bogus errors (it happened before).

There's no way to dump the RA state (except maybe by using the "show syntax tree" command), but I guess it wouldn't be too helpful even if we were to implement it. What would help here would be a LSP trace that we could replay, but most people would probably not want to share it.

The incremental sync might also be a good fuzzing target.

@bjorn3
Copy link
Member

bjorn3 commented Oct 29, 2020

What would help here would be a LSP trace that we could replay, but most people would probably not want to share it.

It could theoretically be combined with a tool to minimize the trace to the bare minimum necessary to reproduce the issue. By for example filtering out read-only requests and collapsing write requests. This tools should work on UTF-16 text to rule out any issues caused by the UTF-16 to UTF-8 conversion. And in addition allowing for an easy way to inspect what the trace contains to see if it contains anything sensitive.

@lnicola
Copy link
Member

lnicola commented Oct 29, 2020

And in addition allowing for an easy way to inspect what the trace contains to see if it contains anything sensitive.

Well, it's a trace of basically every key press in the editor, it's safe to assume that it's sensitive.

@bjorn3
Copy link
Member

bjorn3 commented Oct 29, 2020

That's why I said that there should be a tool to minimize the trace by for example collapsing write requests like key presses. This would remove most of the later deleted code from the trace.

@Timmmm
Copy link
Contributor Author

Timmmm commented Oct 29, 2020

Are you sure you're getting cargo check errors? rust-analyzer runs it as if you would run it from the terminal

Ah in that case no - at the point when RA was reporting errors I ran cargo check manually and it had no errors.

What would help here would be a LSP trace that we could replay, but most people would probably not want to share it.

I think that would be a cool feature. You're right I probably couldn't share it, but if it were a text based format I could maybe edit out all the sensitive bits.

@lnicola
Copy link
Member

lnicola commented Oct 29, 2020

I think that would be a cool feature. You're right I probably couldn't share it, but if it were a text based format I could maybe edit out all the sensitive bits.

We can't load it directly, but you can -- in principle -- enable LSP tracing ("rust-analyzer.trace.server": "verbose") in Code, copy the output to a file, then parse or clean it up.

@Timmmm
Copy link
Contributor Author

Timmmm commented Nov 6, 2020

So I think I was looking in the wrong place at the logs, anyway this happened again and I get this backtrace:

Panic context:
> request: textDocument/codeAction CodeActionParams {
    text_document: TextDocumentIdentifier {
        uri: "file:///redacted/rpc.rs",
    },
    range: Range {
        start: Position {
            line: 0,
            character: 0,
        },
        end: Position {
            line: 0,
            character: 0,
        },
    },
    context: CodeActionContext {
        diagnostics: [],
        only: None,
    },
    work_done_progress_params: WorkDoneProgressParams {
        work_done_token: None,
    },
    partial_result_params: PartialResultParams {
        partial_result_token: None,
    },
}

thread '<unnamed>' panicked at 'var_universe invoked on bound variable', /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/chalk-solve-0.36.0/src/infer.rs:157:41
stack backtrace:
   0: std::panicking::begin_panic
   1: chalk_solve::infer::unify::Unifier<I>::unify_ty_ty
   2: <chalk_ir::Substitution<I> as chalk_ir::zip::Zip<I>>::zip_with
   3: chalk_solve::infer::unify::Unifier<I>::unify_ty_ty
   4: chalk_ir::_DERIVE_chalk_ir_zip_Zip_I_FOR_DomainGoal::<impl chalk_ir::zip::Zip<I> for chalk_ir::DomainGoal<I>>::zip_with
   5: chalk_solve::infer::unify::<impl chalk_solve::infer::InferenceTable<I>>::unify
   6: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::new_with_clause
   7: <chalk_recursive::recursive::Solver<I> as chalk_recursive::solve::SolveDatabase<I>>::solve_goal
   8: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::prove
   9: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::solve
  10: <chalk_recursive::recursive::Solver<I> as chalk_recursive::solve::SolveDatabase<I>>::solve_goal
  11: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::prove
  12: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::solve
  13: <chalk_recursive::recursive::Solver<I> as chalk_recursive::solve::SolveDatabase<I>>::solve_goal
  14: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::prove
  15: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::solve
  16: <chalk_recursive::recursive::Solver<I> as chalk_recursive::solve::SolveDatabase<I>>::solve_goal
  17: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::prove
  18: chalk_recursive::fulfill::Fulfill<I,Solver,Infer>::solve
  19: <chalk_recursive::recursive::Solver<I> as chalk_recursive::solve::SolveDatabase<I>>::solve_goal
  20: <chalk_recursive::recursive::RecursiveSolver<I> as chalk_solve::solve::Solver<I>>::solve_limited
  21: hir_ty::traits::solve::{{closure}}
  22: hir_ty::traits::trait_solve_query
  23: salsa::runtime::Runtime::execute_query_implementation
  24: salsa::derived::slot::Slot<Q,MP>::read_upgrade
  25: salsa::derived::slot::Slot<Q,MP>::read
  26: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
  27: salsa::QueryTable<Q>::get
  28: <DB as hir_ty::db::HirDatabase>::trait_solve::__shim
  29: <DB as hir_ty::db::HirDatabase>::trait_solve
  30: hir_ty::infer::InferenceContext::resolve_ty_as_possible
  31: hir_ty::infer::expr::<impl hir_ty::infer::InferenceContext>::infer_expr_coerce
  32: hir_ty::infer::expr::<impl hir_ty::infer::InferenceContext>::infer_expr_inner
  33: hir_ty::infer::expr::<impl hir_ty::infer::InferenceContext>::infer_expr_coerce
  34: hir_ty::infer::infer_query
  35: salsa::runtime::Runtime::execute_query_implementation
  36: salsa::derived::slot::Slot<Q,MP>::read_upgrade
  37: salsa::derived::slot::Slot<Q,MP>::read
  38: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
  39: <DB as hir_ty::db::HirDatabase>::infer_query::__shim
  40: hir_ty::db::infer_wait
  41: hir::source_analyzer::SourceAnalyzer::new_for_body
  42: hir::semantics::SemanticsImpl::analyze2
  43: hir::semantics::SemanticsImpl::resolve_path
  44: ide_db::defs::NameRefClass::classify
  45: ide_db::search::FindUsages::search
  46: ide_db::search::FindUsages::all
  47: ide::references::find_all_refs
  48: ide::references::rename::rename_with_semantics
  49: <hir_ty::diagnostics::IncorrectCase as ide::diagnostics::fixes::DiagnosticWithFix>::fix
  50: hir_expand::diagnostics::DiagnosticSinkBuilder::on::{{closure}}
  51: hir_expand::diagnostics::DiagnosticSink::_push
  52: hir_ty::diagnostics::decl_check::DeclValidator::validate_item
  53: hir_ty::diagnostics::validate_module_item
  54: hir::code_model::ModuleDef::diagnostics
  55: hir::code_model::Module::diagnostics
  56: ide::diagnostics::diagnostics
  57: ide::Analysis::diagnostics
  58: rust_analyzer::handlers::handle_code_action
  59: <F as threadpool::FnBox>::call_box
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Note that I wasn't actually editing rpc.rs at the time, and haven't for ages. I was editing a different file, and made this edit (the error only appear after I finished this edit - it's shown in this gif because once it appears it stays forever and I made the gif via undo/redo):

analyzer_edit

@lnicola
Copy link
Member

lnicola commented Nov 6, 2020

Yeah, that's a tough one. I think what happens is that:

  1. you typed something and Code requested the diagnostics from us
  2. one of the diagnostics decided that you have an item with the wrong case
    2.1. you might have an #[allow] attribute, but they weren't supported until recently (Check for allow(..) attributes in case check diagnostic #6421)
  3. the diagnostic was too eager and wanted to compute a fix (100% system load on macOS Catalina (too eager incorrect case diagnostic) #6433, Lazy fixes resolving #6466)
  4. to compute the fix, it has to invoke "find all references"
  5. find all references tries to classify each usage (@matklad I wonder if we are doing too much work here, i.e. does classify return more detailed info than needed?)
  6. classify invokes chalk to figure out something about an associated type
  7. chalk panics due to a chalk or chalk/rust-analyzer integration bug (var_universe invoked on bound variable when analysis-stating --with-deps #5495)
  8. the panic should be handled, but I've seen issues like stuck highlighting when/after RA reloads after a hard crash (not a panic)

If you see this again, try to check if it was preceded by a crash.

Fixing the diagnostic would have prevented your issue, but it can crash the same way when you open a file or invoke "find all references" on the problematic item.

@flodiebold flodiebold added the S-unactionable Issue requires feedback, design decisions or is blocked on other work label Dec 21, 2020
@lnicola
Copy link
Member

lnicola commented Jan 22, 2021

#5495 is fixed and our fixes are now lazy, I suppose we can close this.

@lnicola lnicola closed this as completed Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-unactionable Issue requires feedback, design decisions or is blocked on other work
Projects
None yet
Development

No branches or pull requests

5 participants