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

Support in band lifetimes #7416

Closed
jyn514 opened this issue Jan 24, 2021 · 5 comments
Closed

Support in band lifetimes #7416

jyn514 opened this issue Jan 24, 2021 · 5 comments
Labels
E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now

Comments

@jyn514
Copy link
Member

jyn514 commented Jan 24, 2021

This happened trying to rename 'me on https://github.com/jyn514/rust/blob/8edda7e0fb9ae313e47f9f43a07e719b9f0fe9d0/compiler/rustc_infer/src/infer/nll_relate/mod.rs#L818.

thread '<unnamed>' panicked at 'assertion failed: self.hl_range.range.contains_range(hl_range.range)', crates/ide/src/syntax_highlighting/highlights.rs:42:9
stack backtrace:
   0: std::panicking::begin_panic
   1: ide::syntax_highlighting::highlights::Node::add
   2: ide::syntax_highlighting::traverse
   3: ide::syntax_highlighting::highlight
   4: ide::Analysis::highlight_range
   5: rust_analyzer::handlers::handle_semantic_tokens_range
   6: <F as threadpool::FnBox>::call_box
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Panic context:
> request: textDocument/semanticTokens/range SemanticTokensRangeParams {
    work_done_progress_params: WorkDoneProgressParams {
        work_done_token: None,
    },
    partial_result_params: PartialResultParams {
        partial_result_token: None,
    },
    text_document: TextDocumentIdentifier {
        uri: Url {
            scheme: "file",
            host: None,
            port: None,
            path: "/home/joshua/rustc2/compiler/rustc_infer/src/infer/combine.rs",
            query: None,
            fragment: None,
        },
    },
    range: Range {
        start: Position {
            line: 239,
            character: 0,
        },
        end: Position {
            line: 360,
            character: 66,
        },
    },
}
@jyn514
Copy link
Member Author

jyn514 commented Jan 24, 2021

        path: "/home/joshua/rustc2/compiler/rustc_infer/src/infer/combine.rs",

Hmm, that seems wrong.

@jyn514
Copy link
Member Author

jyn514 commented Jan 24, 2021

Actually that might have been an early panic. The error from renaming 'me is

[Error - 3:56:52 PM] Request textDocument/prepareRename failed.
  Message: No references found at position
  Code: -32603 

@Veykril
Copy link
Member

Veykril commented Jan 24, 2021

Reference search fails there because those lifetimes don't have a definition place. As in they aren't listed in the impls generic param list where we currently expect them to be.
So the problem here is in band lifetimes not being supported yet for reference searching(or rather at all I imagine) as those lifetimes therefor have no definition.

So as a workaround when trying to rename these you can temporarily add the definition, rename and then remove the definition I believe 😄

@Veykril Veykril added E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now labels Jan 24, 2021
@Veykril Veykril changed the title Panic trying to rename a lifetime variable Support in band lifetimes Jan 24, 2021
bors bot added a commit that referenced this issue Feb 5, 2021
7505: Widen Highlights root range to covering element r=Veykril a=Veykril

There have been a few issues about/containing spurious syntax highlighting panics, which all seem to come from the `rust_analyzer::handlers::handle_semantic_tokens_range` request, which I believe this to be the cause of as the text range we want to highlight here is currently potentially smaller than that of the covering element, so we might highlight something that is inside the covering element, but outside of the text range we wish to highlight causing the assert to fail.
Unfortunately this isn't really easy to test since I have yet to find a reproducible cause(#7504 doesn't work for me cause I can't seem to checkout the given commit).

See #7504, #7298, #7299 and #7416, all of those contain an assertion failure in syntax highlighting, but only in the range request.

Co-authored-by: Lukas Wirth <[email protected]>
@jhpratt
Copy link
Member

jhpratt commented Dec 17, 2021

In-band lifetimes appear quite likely to be removed from the compiler at this point. Something to keep an eye out for with regard to closing this issue.

@Veykril
Copy link
Member

Veykril commented Feb 22, 2022

This feature is getting removed, so we can close this
rust-lang/rust#91867

@Veykril Veykril closed this as completed Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

3 participants