[Bug] move-analyzer --branch sui-move server crash #1082
Comments
If possible, please specify a sequence of actions that need to be taken to reproduce it. Perhaps you can write a super simple package with one empty module, open it in the IDE and describe what needs to be typed to reproduce this error? |
|
Created a basic repo with an example and more details. https://github.com/thounyy/moveanalyzer-crash |
Thank you! Will get on it shortly! |
|
It has taken me longer that I expected but I am on it. I looked at the repository with repro instructions and I still don't know how to trigger it. In particular:
Ideally, what I would need to repro this is the following.
I think we may have 1 but I don't know 2... |
|
sorry for the late answer. the issue is that it is on any project simple or complex and occurs rapidly at any time when writing any new line of code. |
I can't reproduce it. I am currently actively developing the analyzer, writing a fair amount of Move test code myself, and I did not see it crashing the way you describe. I also tried your example, typing new lines as rapidly as I can, bot at module level (new imports) and inside a function - still no crash. I would really like to help (and make the plugin as good as I can!), and I really grateful for feedback, but without being able to reproduce it, there is not much I can do... I really need a more principled way to reproduce this problem to act on this in a timely manner... |
|
I understand but there's nothing more specific to do, the only thing is that MoveBit extension doesn't have the problem. I searched for more details on the crash, here is the initial messages before it loops over I don't know if it can help but here is the // Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0
use crate::utils::get_loc;
use codespan_reporting::{diagnostic::Severity, files::SimpleFiles};
use lsp_types::{Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, Location, Range};
use move_command_line_common::files::FileHash;
use move_ir_types::location::Loc;
use move_symbol_pool::Symbol;
use std::collections::{BTreeMap, HashMap};
use url::Url;
/// Converts diagnostics from the codespan format to the format understood by the language server.
pub fn lsp_diagnostics(
diagnostics: &Vec<(
codespan_reporting::diagnostic::Severity,
&'static str,
(Loc, String),
Vec<(Loc, String)>,
Vec<String>,
)>,
files: &SimpleFiles<Symbol, String>,
file_id_mapping: &HashMap<FileHash, usize>,
file_name_mapping: &BTreeMap<FileHash, Symbol>,
) -> BTreeMap<Symbol, Vec<Diagnostic>> {
let mut lsp_diagnostics = BTreeMap::new();
for (s, _, (loc, msg), labels, _) in diagnostics {
let fpath = file_name_mapping.get(&loc.file_hash()).unwrap(); // line 28
if let Some(start) = get_loc(&loc.file_hash(), loc.start(), files, file_id_mapping) {
if let Some(end) = get_loc(&loc.file_hash(), loc.end(), files, file_id_mapping) {
let range = Range::new(start, end);
let related_info_opt = if labels.is_empty() {
None
} else {
Some(
labels
.iter()
.filter_map(|(lloc, lmsg)| {
let lstart = get_loc(
&lloc.file_hash(),
lloc.start(),
files,
file_id_mapping,
)?;
let lend =
get_loc(&lloc.file_hash(), lloc.end(), files, file_id_mapping)?;
let lpath = file_name_mapping.get(&lloc.file_hash()).unwrap();
let lpos = Location::new(
Url::from_file_path(lpath.as_str()).unwrap(),
Range::new(lstart, lend),
);
Some(DiagnosticRelatedInformation {
location: lpos,
message: lmsg.to_string(),
})
})
.collect(),
)
};
lsp_diagnostics
.entry(*fpath)
.or_insert_with(Vec::new)
.push(Diagnostic::new(
range,
Some(severity(*s)),
None,
None,
msg.to_string(),
related_info_opt,
None,
));
}
}
}
lsp_diagnostics
}
/// Produces empty diagnostics in the format understood by the language server for all files that
/// the language server is aware of.
pub fn lsp_empty_diagnostics(
file_name_mapping: &BTreeMap<FileHash, Symbol>,
) -> BTreeMap<Symbol, Vec<Diagnostic>> {
let mut lsp_diagnostics = BTreeMap::new();
for n in file_name_mapping.values() {
lsp_diagnostics.insert(*n, vec![]);
}
lsp_diagnostics
}
/// Converts diagnostic severity level from the codespan format to the format understood by the
/// language server.
fn severity(s: Severity) -> DiagnosticSeverity {
match s {
Severity::Bug => DiagnosticSeverity::Error,
Severity::Error => DiagnosticSeverity::Error,
Severity::Warning => DiagnosticSeverity::Warning,
Severity::Note => DiagnosticSeverity::Information,
Severity::Help => DiagnosticSeverity::Hint,
}
} |
Thank you! It certainly helps as it gives me an idea on what part of the implementation to look at. It's a very strange one, because:
I will look into it, but I was hoping that you could help me debug this a bit further. If you could modify the diagnostics.rs file to include some extra debugging info, rebuild move-analyzer, and include the more verbose output, it would help even more. I am thinking something along changing line 28 from this into this |
|
I don't know if it was expected but by modifying the line 28 like you proposed then rebuilding and installing move-analyzer from the modified repo I don't get the error anymore. I'll let you know whether it happens again or not with this config. |
The error (crash) will indeed not happen anymore because we hit |
|
oh right, well I didn't see the messages in the output console so I guess it never goes the // let fpath = file_name_mapping.get(&loc.file_hash()).unwrap();
let Some(fpath) = file_name_mapping.get(&loc.file_hash()) else {
continue;
};
eprintln!("MAPPING FILE ERROR");
eprintln!("LOC: {:?}", loc);
eprintln!("MSG: {msg}");I moved them outside the else and it shows: |
This unfortunately does not help any further... The motivation behind the change I proposed was to capture error-related information right before the error actually happens. The In other words, I did not really change anything in terms of the error happening or not - you should see the "MAPPING ERROR..." messages in the same situations that you saw the crash earlier. |
|
Indeed, it makes sense. Since I don't get the error with the eprintln!("MAPPING FILE ERROR");
eprintln!("LOC: {:?}", loc);
eprintln!("MSG: {msg}");
let fpath = file_name_mapping.get(&loc.file_hash()).unwrap();here is what I get before the error: |
## Description This PR adds support for virtual file system, with the intention of allowing the compiler to build a package from files kept by the IDE in memory (this feature is upcoming). It also removes a certain awkwardness from the compiler implementation where interface files had to be built in a temporary directory (now they are built in an in-memory file system). It also fixes an existing bug (move-language/move#1082) where source files used in the symbolicator (obtained from resolution graph) and source files used by the compiler could be modified between the two uses resulting in different file hashes which can ultimately lead to crash when translating diagnostics (generated by the compiler and using "compiler file hashes") using symbolicator source files (and "symbolicator file hashes") ## Test Plan All existing tests must pass. Also, manually tested the IDE no longer needs file saves to reflect changes in the file (e.g., compiler diagnostics appear as the code is being typed).
## Description This PR adds support for virtual file system, with the intention of allowing the compiler to build a package from files kept by the IDE in memory (this feature is upcoming). It also removes a certain awkwardness from the compiler implementation where interface files had to be built in a temporary directory (now they are built in an in-memory file system). It also fixes an existing bug (move-language/move#1082) where source files used in the symbolicator (obtained from resolution graph) and source files used by the compiler could be modified between the two uses resulting in different file hashes which can ultimately lead to crash when translating diagnostics (generated by the compiler and using "compiler file hashes") using symbolicator source files (and "symbolicator file hashes") ## Test Plan All existing tests must pass. Also, manually tested the IDE no longer needs file saves to reflect changes in the file (e.g., compiler diagnostics appear as the code is being typed).
馃悰 Bug
To reproduce
Any Sui Move module, sometimes when adding:
But I think it happens whenever I write a new line of code in the middle of a function or module, and it necessitates a comma but it crashes before I add it.
Stack trace/error message
Expected Behavior
Keep highlighting the errors and warnings instead of getting stuck on an inexistant comma. Like the sui-move plug-in from movebit.
System information
Please complete the following information:
Additional context
Feel free to guide me to give more details.
The text was updated successfully, but these errors were encountered: