[move][move-ide] Revise how IDE information is recorded in the compiler #17864
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
| #[derive(Debug, Clone)] | ||
| pub struct ExpEntry { | ||
| /// Location of the recorded info | ||
| pub loc: Loc, |
There was a problem hiding this comment.
Is this a different location than the one used as key of the exp_info map (e.g., indicates a different, more precise location inside of the expression)?
There was a problem hiding this comment.
No, it's just duplicated here for sanity.
| @@ -3769,12 +3765,12 @@ fn method_call( | |||
| edotted.for_autocomplete = true; | |||
| let err_ty = context.error_type(loc); | |||
| let dot_output = | |||
| resolve_exp_dotted(context, DottedUsage::Borrow(false), loc, edotted); | |||
| resolve_exp_dotted(context, DottedUsage::Borrow(false), edotted.loc, edotted); | |||
There was a problem hiding this comment.
Is this related or unrelated fix?
There was a problem hiding this comment.
All of the span stuff is quite related. I'll provide a write-up in the mian PR when I publish it for review.
| } | ||
|
|
||
| /// For use in the compiler, to ease recording IDE information. | ||
| pub enum ExpInfo { |
There was a problem hiding this comment.
It looks like ExpEntry and ExpInfo encode the same type of information. Can we just use one rather than using ExpInfo in the compiler and (at least seemingly) translating it almost immediately into ExpEntry?
There was a problem hiding this comment.
Unfortunately, this is likely the cleanest thing we can do. Since the actual information live on the CompilationEnv take a Diagnostics to report ICEs, the only alternative would be to re-expose the entire interface for every information form on that environment, e.g.,
pub fun set_ide_exp_macro_call_info(&mut self, call_info: ide::MacroCallInfo) {
...
}
pub fun set_ide_exp_expanded_lambda(&mut self) {
...
}
pub fun set_ide_exp_autocomplete_dot_information(&mut self) {
...
}
The current approach avoids this by essentially baking this dispatch into the ExpInfo. I'm happy to change this if you want, but I think the overall usage and maintenance will become clumsier.
cgswords
changed the title
cgswords
force-pushed
the
cgswords/ide_info
branch
from
dd98104
to
1d9c5b0
Compare
cgswords
force-pushed
the
cgswords/ide_info
branch
from
53912a3
to
77a83b2
Compare
…7889) ## Description This adds autocomplete information collection to the `IDEInfo` form. ## Test plan Testing is difficult without the analyzer calling it with a location, but I anticipate working with Adam on that over the next few days. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
Description
This PR moves how we record IDE information to communicate it to the Move Analyzer. It is now accrued on the
CompilationEnv(modulo some care in typing to handle type elaboration) and can be added through methods there. The goal is to set up an extensible way to track IDE information in the future, tying it to location information.This change has also caused us to change how we compute spans for dotted expression terms: previously, we would make the expression with the call's span in order to report failures to autoborrow to point at the call site (instead of just the subject term). This caused recurring through location information in the analyzer to enter an infinite loop in the macro case, as the subject term shared the location of the overall macro call and recursion into the subject term on the macro information looped forever. This diff introduces a solution to this issue by more-carefully computing spans for subject terms so that they remain pointing at the actual subject term, and plumbs the call location through dotted expression handling to retain good error reporting for the method case. It also introduces a very small change to handle location reporting for
*/&/&mutusage for type errors.Test plan
All tests still pass.
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.