Skip to content

Commit 32bf1f1

Browse files
committed
[mlir:LSP] Improve conversion between SourceMgr and LSP locations
SourceMgr generally uses 1-based locations, whereas the LSP is zero based. This commit corrects this conversion and also enhances the conversion from SMLoc to SMRange to support string tokens. Differential Revision: https://reviews.llvm.org/D124584
1 parent 9613a85 commit 32bf1f1

File tree

6 files changed

+76
-17
lines changed

6 files changed

+76
-17
lines changed

mlir/lib/Parser/AsmParserState.cpp

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "mlir/Parser/AsmParserState.h"
1010
#include "mlir/IR/Operation.h"
1111
#include "mlir/IR/SymbolTable.h"
12+
#include "llvm/ADT/StringExtras.h"
1213

1314
using namespace mlir;
1415

@@ -121,18 +122,52 @@ auto AsmParserState::getOpDef(Operation *op) const
121122
: &*impl->operations[it->second];
122123
}
123124

125+
/// Lex a string token whose contents start at the given `curPtr`. Returns the
126+
/// position at the end of the string, after a terminal or invalid character
127+
/// (e.g. `"` or `\0`).
128+
static const char *lexLocStringTok(const char *curPtr) {
129+
while (char c = *curPtr++) {
130+
// Check for various terminal characters.
131+
if (StringRef("\"\n\v\f").contains(c))
132+
return curPtr;
133+
134+
// Check for escape sequences.
135+
if (c == '\\') {
136+
// Check a few known escapes and \xx hex digits.
137+
if (*curPtr == '"' || *curPtr == '\\' || *curPtr == 'n' || *curPtr == 't')
138+
++curPtr;
139+
else if (llvm::isHexDigit(*curPtr) && llvm::isHexDigit(curPtr[1]))
140+
curPtr += 2;
141+
else
142+
return curPtr;
143+
}
144+
}
145+
146+
// If we hit this point, we've reached the end of the buffer. Update the end
147+
// pointer to not point past the buffer.
148+
return curPtr - 1;
149+
}
150+
124151
SMRange AsmParserState::convertIdLocToRange(SMLoc loc) {
125152
if (!loc.isValid())
126153
return SMRange();
154+
const char *curPtr = loc.getPointer();
127155

128-
// Return if the given character is a valid identifier character.
129-
auto isIdentifierChar = [](char c) {
130-
return isalnum(c) || c == '$' || c == '.' || c == '_' || c == '-';
131-
};
156+
// Check if this is a string token.
157+
if (*curPtr == '"') {
158+
curPtr = lexLocStringTok(curPtr + 1);
159+
160+
// Otherwise, default to handling an identifier.
161+
} else {
162+
// Return if the given character is a valid identifier character.
163+
auto isIdentifierChar = [](char c) {
164+
return isalnum(c) || c == '$' || c == '.' || c == '_' || c == '-';
165+
};
166+
167+
while (*curPtr && isIdentifierChar(*(++curPtr)))
168+
continue;
169+
}
132170

133-
const char *curPtr = loc.getPointer();
134-
while (*curPtr && isIdentifierChar(*(++curPtr)))
135-
continue;
136171
return SMRange(loc, SMLoc::getFromPointer(curPtr));
137172
}
138173

mlir/lib/Tools/lsp-server-support/Protocol.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ struct Position {
279279
/// source manager.
280280
SMLoc getAsSMLoc(llvm::SourceMgr &mgr) const {
281281
return mgr.FindLocForLineAndColumn(mgr.getMainFileID(), line + 1,
282-
character);
282+
character + 1);
283283
}
284284
};
285285

mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ static Optional<lsp::Location> getLocationFromLoc(FileLineColLoc loc) {
3030

3131
lsp::Position position;
3232
position.line = loc.getLine() - 1;
33-
position.character = loc.getColumn();
33+
position.character = loc.getColumn() ? loc.getColumn() - 1 : 0;
3434
return lsp::Location{*sourceURI, lsp::Range(position)};
3535
}
3636

mlir/lib/Tools/mlir-pdll-lsp-server/PDLLServer.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -983,9 +983,6 @@ PDLDocument::getCodeCompletion(const lsp::URIForFile &uri,
983983
if (!posLoc.isValid())
984984
return lsp::CompletionList();
985985

986-
// Adjust the position one further to after the completion trigger token.
987-
posLoc = SMLoc::getFromPointer(posLoc.getPointer() + 1);
988-
989986
// To perform code completion, we run another parse of the module with the
990987
// code completion context provided.
991988
ods::Context tmpODSContext;
@@ -1132,9 +1129,6 @@ lsp::SignatureHelp PDLDocument::getSignatureHelp(const lsp::URIForFile &uri,
11321129
if (!posLoc.isValid())
11331130
return lsp::SignatureHelp();
11341131

1135-
// Adjust the position one further to after the completion trigger token.
1136-
posLoc = SMLoc::getFromPointer(posLoc.getPointer() + 1);
1137-
11381132
// To perform code completion, we run another parse of the module with the
11391133
// code completion context provided.
11401134
ods::Context tmpODSContext;

mlir/test/mlir-lsp-server/diagnostics.test

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,37 @@
1919
// CHECK-NEXT: "line": 0
2020
// CHECK-NEXT: },
2121
// CHECK-NEXT: "start": {
22-
// CHECK-NEXT: "character": 11,
22+
// CHECK-NEXT: "character": 10,
23+
// CHECK-NEXT: "line": 0
24+
// CHECK-NEXT: }
25+
// CHECK-NEXT: },
26+
// CHECK-NEXT: "severity": 1,
27+
// CHECK-NEXT: "source": "mlir"
28+
// CHECK-NEXT: }
29+
// CHECK-NEXT: ],
30+
// CHECK-NEXT: "uri": "test:///foo.mlir",
31+
// CHECK-NEXT: "version": 1
32+
// CHECK-NEXT: }
33+
// -----
34+
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
35+
"uri":"test:///foo.mlir",
36+
"languageId":"mlir",
37+
"version":1,
38+
"text":"\"\""
39+
}}}
40+
// CHECK: "method": "textDocument/publishDiagnostics",
41+
// CHECK-NEXT: "params": {
42+
// CHECK-NEXT: "diagnostics": [
43+
// CHECK-NEXT: {
44+
// CHECK-NEXT: "category": "Parse Error",
45+
// CHECK-NEXT: "message": "empty operation name is invalid",
46+
// CHECK-NEXT: "range": {
47+
// CHECK-NEXT: "end": {
48+
// CHECK-NEXT: "character": 2,
49+
// CHECK-NEXT: "line": 0
50+
// CHECK-NEXT: },
51+
// CHECK-NEXT: "start": {
52+
// CHECK-NEXT: "character": 0,
2353
// CHECK-NEXT: "line": 0
2454
// CHECK-NEXT: }
2555
// CHECK-NEXT: },

mlir/test/mlir-pdll-lsp-server/definition-split-file.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// -----
1414
{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{
1515
"textDocument":{"uri":"test:///foo.pdll"},
16-
"position":{"line":3,"character":12}
16+
"position":{"line":3,"character":11}
1717
}}
1818
// CHECK: "id": 1
1919
// CHECK-NEXT: "jsonrpc": "2.0",

0 commit comments

Comments
 (0)