-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[mlir][lsp] Downgrade unregistered dialect error. #143346
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
base: main
Are you sure you want to change the base?
Conversation
Currently its an error, but its not a user error akin to invalid syntax or unverified IR. Rather treat as warning so that user is informed, but not error.
@llvm/pr-subscribers-mlir-core Author: Jacques Pienaar (jpienaar) ChangesCurrently its an error, but its not a user error akin to invalid syntax or unverified IR. Rather treat as warning so that user is informed, but not error. Full diff: https://github.com/llvm/llvm-project/pull/143346.diff 2 Files Affected:
diff --git a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
index 61987525a5ca5..e4fe23210f781 100644
--- a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
@@ -231,6 +231,17 @@ static lsp::Diagnostic getLspDiagnoticFromDiag(llvm::SourceMgr &sourceMgr,
}
lspDiag.message = diag.str();
+ // Downgrade errors related to unregistered dialects. We want to be able to
+ // provide the user with headsup about why the file didn't parse, but it is
+ // not an error in the same way invalid syntax or op that failed verification
+ // is. Chose to make it a warning rather than information as it could be due
+ // to typo (and so addressable by the user).
+ if (lspDiag.severity == lsp::DiagnosticSeverity::Error &&
+ lspDiag.message.starts_with("Dialect `") &&
+ StringRef(lspDiag.message).contains("not found for custom op")) {
+ lspDiag.severity = lsp::DiagnosticSeverity::Warning;
+ }
+
// Attach any notes to the main diagnostic as related information.
std::vector<lsp::DiagnosticRelatedInformation> relatedDiags;
for (Diagnostic ¬e : diag.getNotes()) {
diff --git a/mlir/test/mlir-lsp-server/diagnostics.test b/mlir/test/mlir-lsp-server/diagnostics.test
index 99edd11b574f5..8ee9be61d3214 100644
--- a/mlir/test/mlir-lsp-server/diagnostics.test
+++ b/mlir/test/mlir-lsp-server/diagnostics.test
@@ -61,6 +61,28 @@
// CHECK-NEXT: "version": 1
// CHECK-NEXT: }
// -----
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
+ "uri":"test:///foo.mlir",
+ "languageId":"mlir",
+ "version":1,
+ "text":"unregistered.func.func ()"
+}}}
+// CHECK: "method": "textDocument/publishDiagnostics",
+// CHECK-NEXT: "params": {
+// CHECK-NEXT: "diagnostics": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "category": "Parse Error",
+// Note: If the next lines need change, please update the corresponding logic
+// in MLIRServer.cpp to ensure the severity is still set as expected.
+// CHECK-NEXT: "message": "Dialect `unregistered' not found for custom
+// CHECK: "severity": 2,
+// CHECK-NEXT: "source": "mlir"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "uri": "test:///foo.mlir",
+// CHECK-NEXT: "version": 1
+// CHECK-NEXT: }
+// -----
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
// -----
{"jsonrpc":"2.0","method":"exit"}
|
@llvm/pr-subscribers-mlir Author: Jacques Pienaar (jpienaar) ChangesCurrently its an error, but its not a user error akin to invalid syntax or unverified IR. Rather treat as warning so that user is informed, but not error. Full diff: https://github.com/llvm/llvm-project/pull/143346.diff 2 Files Affected:
diff --git a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
index 61987525a5ca5..e4fe23210f781 100644
--- a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
@@ -231,6 +231,17 @@ static lsp::Diagnostic getLspDiagnoticFromDiag(llvm::SourceMgr &sourceMgr,
}
lspDiag.message = diag.str();
+ // Downgrade errors related to unregistered dialects. We want to be able to
+ // provide the user with headsup about why the file didn't parse, but it is
+ // not an error in the same way invalid syntax or op that failed verification
+ // is. Chose to make it a warning rather than information as it could be due
+ // to typo (and so addressable by the user).
+ if (lspDiag.severity == lsp::DiagnosticSeverity::Error &&
+ lspDiag.message.starts_with("Dialect `") &&
+ StringRef(lspDiag.message).contains("not found for custom op")) {
+ lspDiag.severity = lsp::DiagnosticSeverity::Warning;
+ }
+
// Attach any notes to the main diagnostic as related information.
std::vector<lsp::DiagnosticRelatedInformation> relatedDiags;
for (Diagnostic ¬e : diag.getNotes()) {
diff --git a/mlir/test/mlir-lsp-server/diagnostics.test b/mlir/test/mlir-lsp-server/diagnostics.test
index 99edd11b574f5..8ee9be61d3214 100644
--- a/mlir/test/mlir-lsp-server/diagnostics.test
+++ b/mlir/test/mlir-lsp-server/diagnostics.test
@@ -61,6 +61,28 @@
// CHECK-NEXT: "version": 1
// CHECK-NEXT: }
// -----
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
+ "uri":"test:///foo.mlir",
+ "languageId":"mlir",
+ "version":1,
+ "text":"unregistered.func.func ()"
+}}}
+// CHECK: "method": "textDocument/publishDiagnostics",
+// CHECK-NEXT: "params": {
+// CHECK-NEXT: "diagnostics": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "category": "Parse Error",
+// Note: If the next lines need change, please update the corresponding logic
+// in MLIRServer.cpp to ensure the severity is still set as expected.
+// CHECK-NEXT: "message": "Dialect `unregistered' not found for custom
+// CHECK: "severity": 2,
+// CHECK-NEXT: "source": "mlir"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "uri": "test:///foo.mlir",
+// CHECK-NEXT: "version": 1
+// CHECK-NEXT: }
+// -----
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
// -----
{"jsonrpc":"2.0","method":"exit"}
|
Currently its an error, but its not a user error akin to invalid syntax or unverified IR. Rather treat as warning so that user is informed, but not error.