Skip to content

Commit 6164a19

Browse files
committed
[circt-verilog-lsp] Add -C command line option
This PR extends the CIRCT Verilog LSP server with support for project command files (`-C` / `--command-file`) and improves robustness around source locations and diagnostics. - **Command file support** - Added `commandFiles` option to `VerilogServerOptions`. - Extended the CLI with `-C`/`--command-file` flags to pass one or more command files. - Each buffer’s `Driver` now processes command files, filtering out the current main buffer to avoid duplication. - Implemented `removeFileToTemp` helper to strip the main file from command files and materialize a temporary version. - **Source location handling** - Improved absolute/real path resolution when constructing LSP URIs. - Fallbacks added for when `slang::SourceManager` has no full path info. - **Compilation setup** - Ensure all definitions in the buffer/project scope are treated as top modules. - **Indexing improvements** - Skip top instances not defined in the current file. - Added guards against invalid or empty ranges (especially macro expansions).
1 parent 6093d7b commit 6164a19

File tree

7 files changed

+284
-17
lines changed

7 files changed

+284
-17
lines changed

include/circt/Tools/circt-verilog-lsp-server/CirctVerilogLspServerMain.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,17 @@ namespace circt {
3535
namespace lsp {
3636
struct VerilogServerOptions {
3737
VerilogServerOptions(const std::vector<std::string> &libDirs,
38-
const std::vector<std::string> &extraSourceLocationDirs)
39-
: libDirs(libDirs), extraSourceLocationDirs(extraSourceLocationDirs) {}
38+
const std::vector<std::string> &extraSourceLocationDirs,
39+
const std::vector<std::string> &commandFiles)
40+
: libDirs(libDirs), extraSourceLocationDirs(extraSourceLocationDirs),
41+
commandFiles(commandFiles) {}
4042
/// Additional list of RTL directories to search.
4143
const std::vector<std::string> &libDirs;
42-
4344
/// Additional list of external source directories to search.
4445
const std::vector<std::string> &extraSourceLocationDirs;
46+
/// Additional list of command files that reference dependencies of the
47+
/// project.
48+
const std::vector<std::string> &commandFiles;
4549
};
4650
// namespace lsp
4751

lib/Tools/circt-verilog-lsp-server/LSPServer.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "LSPServer.h"
1010
#include "VerilogServerImpl/VerilogServer.h"
11+
#include "llvm/Support/JSON.h"
1112
#include "llvm/Support/LSP/Protocol.h"
1213
#include "llvm/Support/LSP/Transport.h"
1314
#include <optional>

lib/Tools/circt-verilog-lsp-server/VerilogServerImpl/VerilogServer.cpp

Lines changed: 193 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@
2323
#include "mlir/IR/MLIRContext.h"
2424
#include "slang/ast/ASTVisitor.h"
2525
#include "slang/ast/Compilation.h"
26+
#include "slang/ast/Scope.h"
27+
#include "slang/ast/symbols/CompilationUnitSymbols.h"
2628
#include "slang/diagnostics/DiagnosticClient.h"
2729
#include "slang/diagnostics/Diagnostics.h"
2830
#include "slang/driver/Driver.h"
31+
#include "slang/syntax/AllSyntax.h"
2932
#include "slang/syntax/SyntaxTree.h"
3033
#include "slang/text/SourceLocation.h"
3134
#include "slang/text/SourceManager.h"
@@ -42,6 +45,8 @@
4245
#include "llvm/Support/SMLoc.h"
4346
#include "llvm/Support/SourceMgr.h"
4447

48+
#include <filesystem>
49+
#include <fstream>
4550
#include <memory>
4651
#include <optional>
4752
#include <string>
@@ -247,13 +252,107 @@ void LSPDiagnosticClient::report(const slang::ReportedDiagnostic &slangDiag) {
247252
// VerilogDocument
248253
//===----------------------------------------------------------------------===//
249254

255+
// Filter out the main buffer file from the command file list, if it is in
256+
// there.
257+
static inline std::string removeFileToTemp(const std::string &cmdfileStr,
258+
const std::string &targetAbsStr) {
259+
namespace fs = std::filesystem;
260+
261+
fs::path cmdfile(cmdfileStr);
262+
fs::path targetAbs(targetAbsStr);
263+
const fs::path base = cmdfile.parent_path();
264+
std::error_code ec;
265+
266+
// Normalize target
267+
fs::path target = fs::weakly_canonical(targetAbs, ec);
268+
if (ec) {
269+
ec.clear();
270+
target = targetAbs.lexically_normal();
271+
}
272+
273+
std::ifstream in(cmdfile);
274+
if (!in) {
275+
// return the original path so the caller still has a valid file to use
276+
return cmdfileStr;
277+
}
278+
279+
std::string line, outbuf;
280+
bool removed = false;
281+
282+
while (std::getline(in, line)) {
283+
std::string s = line;
284+
s.erase(0, s.find_first_not_of(" \t\r\n")); // left-trim
285+
286+
// Keep plusargs
287+
if (!s.empty() &&
288+
(s.rfind("+incdir+", 0) == 0 || s.rfind("+define+", 0) == 0 ||
289+
s.rfind("-I", 0) == 0 || s.rfind("-D", 0) == 0)) {
290+
outbuf += line;
291+
outbuf.push_back('\n');
292+
continue;
293+
}
294+
// Preserve blank lines
295+
if (s.empty()) {
296+
outbuf.push_back('\n');
297+
continue;
298+
}
299+
300+
// Treat everything else as a path (relative entries resolved vs. cmdfile
301+
// dir)
302+
fs::path candRel = s;
303+
fs::path candAbs = candRel.is_absolute() ? candRel : (base / candRel);
304+
305+
fs::path cand = fs::weakly_canonical(candAbs, ec);
306+
if (ec) {
307+
ec.clear();
308+
cand = candAbs.lexically_normal();
309+
}
310+
311+
if (cand == target) {
312+
removed = true; // skip writing this line
313+
} else {
314+
outbuf += line;
315+
outbuf.push_back('\n');
316+
}
317+
}
318+
319+
if (!removed) {
320+
// Nothing changed: return the original commandfile path
321+
return cmdfileStr;
322+
}
323+
// Write updated content to a temp file next to the original
324+
fs::path tmp = base / (cmdfile.filename().string() + ".tmp");
325+
std::ofstream out(tmp, std::ios::trunc);
326+
out << outbuf;
327+
return tmp.string();
328+
}
329+
330+
static inline std::string removeFileToTemp(llvm::StringRef cmdfile,
331+
llvm::StringRef target_abs) {
332+
return removeFileToTemp(cmdfile.str(), target_abs.str());
333+
}
334+
250335
VerilogDocument::VerilogDocument(
251336
VerilogServerContext &context, const llvm::lsp::URIForFile &uri,
252337
StringRef contents, std::vector<llvm::lsp::Diagnostic> &diagnostics)
253338
: globalContext(context), index(*this), uri(uri) {
254339
unsigned int bufferId;
340+
341+
llvm::SmallString<256> canonPath(uri.file());
342+
if (std::error_code ec = llvm::sys::fs::real_path(uri.file(), canonPath))
343+
canonPath = uri.file(); // fall back, but try to keep it absolute
344+
345+
// --- Apply project command files (the “-C”s) to this per-buffer driver ---
346+
for (const std::string &cmdFile : context.options.commandFiles) {
347+
if (!driver.processCommandFiles(removeFileToTemp(cmdFile, canonPath), false,
348+
true)) {
349+
circt::lsp::Logger::error(Twine("Failed to open command file ") +
350+
cmdFile);
351+
}
352+
}
353+
255354
if (auto memBufferOwn =
256-
llvm::MemoryBuffer::getMemBufferCopy(contents, uri.file())) {
355+
llvm::MemoryBuffer::getMemBufferCopy(contents, canonPath)) {
257356

258357
bufferId = sourceMgr.AddNewSourceBuffer(std::move(memBufferOwn), SMLoc());
259358
} else {
@@ -303,6 +402,18 @@ VerilogDocument::VerilogDocument(
303402
return;
304403
}
305404

405+
compilation = driver.createCompilation();
406+
if (failed(compilation))
407+
return;
408+
409+
std::vector<std::string> topModules;
410+
for (const auto *defs : (*compilation)->getDefinitions())
411+
topModules.emplace_back(defs->name);
412+
413+
// Make sure that all possible definitions in the file / project scope are
414+
// topModules!
415+
driver.options.topModules = topModules;
416+
306417
compilation = driver.createCompilation();
307418
if (failed(compilation))
308419
return;
@@ -321,19 +432,28 @@ VerilogDocument::getLspLocation(slang::SourceLocation loc) const {
321432
auto line = slangSourceManager.getLineNumber(loc) - 1;
322433
auto column = slangSourceManager.getColumnNumber(loc) - 1;
323434
auto it = bufferIDMap.find(loc.buffer().getId());
324-
// Check if the current buffer is the main file.
325435
if (it != bufferIDMap.end() && it->second == sourceMgr.getMainFileID())
326436
return llvm::lsp::Location(uri, llvm::lsp::Range(Position(line, column)));
327437

328-
// Otherwise, construct URI from slang source manager.
329-
auto fileName = slangSourceManager.getFileName(loc);
330-
auto loc = llvm::lsp::URIForFile::fromFile(fileName);
331-
if (auto e = loc.takeError())
332-
return llvm::lsp::Location();
333-
return llvm::lsp::Location(loc.get(),
334-
llvm::lsp::Range(Position(line, column)));
335-
}
438+
llvm::StringRef fileName = slangSourceManager.getFileName(loc);
439+
// Ensure absolute path for LSP:
440+
llvm::SmallString<256> abs(fileName);
441+
if (!llvm::sys::path::is_absolute(abs)) {
442+
// Try realPath first
443+
if (std::error_code ec = llvm::sys::fs::real_path(fileName, abs)) {
444+
// Fallback: make it absolute relative to the process CWD
445+
llvm::sys::fs::current_path(abs); // abs = CWD
446+
llvm::sys::path::append(abs, fileName);
447+
}
448+
}
336449

450+
if (auto uriOrErr = llvm::lsp::URIForFile::fromFile(abs)) {
451+
return llvm::lsp::Location(*uriOrErr,
452+
llvm::lsp::Range(Position(line, column)));
453+
}
454+
// If we can't form a URI, return empty (avoids crashing)
455+
return llvm::lsp::Location();
456+
}
337457
return llvm::lsp::Location();
338458
}
339459

@@ -357,8 +477,24 @@ llvm::SMLoc VerilogDocument::getSMLoc(slang::SourceLocation loc) {
357477
auto bufferIDMapIt = bufferIDMap.find(bufferID);
358478
if (bufferIDMapIt == bufferIDMap.end()) {
359479
// If not, open the source file and add it to the LLVM source manager.
360-
auto path = getSlangSourceManager().getFullPath(loc.buffer());
361-
auto memBuffer = llvm::MemoryBuffer::getFile(path.string());
480+
const slang::SourceManager &ssm = getSlangSourceManager();
481+
auto path = ssm.getFullPath(loc.buffer());
482+
483+
std::string name;
484+
if (path.empty()) {
485+
// Get filename from slang source manager - might've been loaded already!
486+
llvm::StringRef fname = ssm.getFileName(loc);
487+
llvm::SmallString<256> realPath;
488+
if (!llvm::sys::fs::real_path(fname, realPath)) {
489+
name = realPath.str().str();
490+
} else {
491+
name = fname.str();
492+
}
493+
} else {
494+
name = path.string();
495+
}
496+
497+
auto memBuffer = llvm::MemoryBuffer::getFile(name);
362498
if (!memBuffer) {
363499
circt::lsp::Logger::error(
364500
"Failed to open file: " + path.filename().string() +
@@ -523,6 +659,13 @@ struct VerilogIndexer : slang::ast::ASTVisitor<VerilogIndexer, true, true> {
523659
return;
524660
assert(range.start().valid() && range.end().valid() &&
525661
"range must be valid");
662+
663+
// TODO: This implementation does not handle expanded MACROs. Return
664+
// instead.
665+
if (range.start() >= range.end()) {
666+
return;
667+
}
668+
526669
index.insertSymbol(symbol, range, isDefinition);
527670
}
528671

@@ -557,6 +700,30 @@ struct VerilogIndexer : slang::ast::ASTVisitor<VerilogIndexer, true, true> {
557700
visitDefault(expr);
558701
}
559702

703+
void visit(const slang::ast::InstanceSymbol &expr) {
704+
auto *def = &expr.getDefinition();
705+
if (!def)
706+
return;
707+
708+
// Add the module definition
709+
insertSymbol(def, def->location, /*isDefinition=*/true);
710+
711+
// Walk up the syntax tree until we hit the type token;
712+
// Link that token back to the instance declaration.
713+
if (auto *hierInst =
714+
expr.getSyntax()
715+
->as_if<slang::syntax::HierarchicalInstanceSyntax>())
716+
if (auto *modInst =
717+
hierInst->parent
718+
->as_if<slang::syntax::HierarchyInstantiationSyntax>())
719+
if (modInst->type)
720+
insertSymbol(def, modInst->type.location(), false);
721+
722+
// Link the module instance name back to the module definition
723+
insertSymbol(def, expr.location, /*isDefinition=*/false);
724+
visitDefault(expr);
725+
}
726+
560727
void visit(const slang::ast::VariableDeclStatement &expr) {
561728
insertSymbol(&expr.symbol, expr.sourceRange, /*isDefinition=*/true);
562729
visitDefault(expr);
@@ -580,7 +747,13 @@ struct VerilogIndexer : slang::ast::ASTVisitor<VerilogIndexer, true, true> {
580747
void VerilogIndex::initialize(slang::ast::Compilation &compilation) {
581748
const auto &root = compilation.getRoot();
582749
VerilogIndexer visitor(*this);
750+
583751
for (auto *inst : root.topInstances) {
752+
753+
// Skip all modules, interfaces, etc. that are not defined in this files
754+
if (!(document.getLspLocation(inst->location).uri == document.getURI()))
755+
continue;
756+
584757
// Visit the body of the instance.
585758
inst->body.visit(visitor);
586759

@@ -752,10 +925,17 @@ void circt::lsp::VerilogServer::findReferencesOf(
752925
void VerilogIndex::insertSymbol(const slang::ast::Symbol *symbol,
753926
slang::SourceRange from, bool isDefinition) {
754927
assert(from.start().valid() && from.end().valid());
928+
929+
// TODO: Currently doesn't handle expanded macros
930+
if (!from.start().valid() || !from.end().valid() ||
931+
from.start() >= from.end())
932+
return;
933+
755934
const char *startLoc = getDocument().getSMLoc(from.start()).getPointer();
756935
const char *endLoc = getDocument().getSMLoc(from.end()).getPointer() + 1;
757-
if (!startLoc || !endLoc)
936+
if (!startLoc || !endLoc || startLoc >= endLoc)
758937
return;
938+
759939
assert(startLoc && endLoc);
760940

761941
if (startLoc != endLoc && !intervalMap.overlaps(startLoc, endLoc)) {
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// RUN: cd %S && circt-verilog-lsp-server -lit-test -C %S/include/filelist.f < %s | FileCheck %s
2+
// REQUIRES: slang
3+
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"verilog","capabilities":{},"trace":"off"}}
4+
// -----
5+
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
6+
"uri":"test:///foo.sv",
7+
"languageId":"verilog",
8+
"version":1,
9+
"text":"module test(output out\n);ifdef#()i_dut(.out(out));\nendmodule"
10+
}}}
11+
// -----
12+
// Find definition of `i_dut`
13+
{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{
14+
"textDocument":{"uri":"test:///foo.sv"},
15+
"position":{"line":1,"character":12}
16+
}}
17+
// -----
18+
// CHECK: "id": 1,
19+
// CHECK-NEXT: "jsonrpc": "2.0",
20+
// CHECK-NEXT: "result": [
21+
// CHECK-NEXT: {
22+
// CHECK-NEXT: "range": {
23+
// CHECK-NEXT: "end": {
24+
// CHECK-NEXT: "character": 12,
25+
// CHECK-NEXT: "line": 0
26+
// CHECK-NEXT: },
27+
// CHECK-NEXT: "start": {
28+
// CHECK-NEXT: "character": 7,
29+
// CHECK-NEXT: "line": 0
30+
// CHECK-NEXT: }
31+
// CHECK-NEXT: },
32+
// CHECK-NEXT: "uri": "file:///{{.*}}/include/ifdef.sv"
33+
// CHECK-NEXT: }
34+
// CHECK-NEXT: ]
35+
// CHECK-NEXT:}
36+
// -----
37+
// Find definition of `ifdef`
38+
{"jsonrpc":"2.0","id":2,"method":"textDocument/definition","params":{
39+
"textDocument":{"uri":"test:///foo.sv"},
40+
"position":{"line":1,"character":4}
41+
}}
42+
// -----
43+
// CHECK: "id": 2,
44+
// CHECK-NEXT: "jsonrpc": "2.0",
45+
// CHECK-NEXT: "result": [
46+
// CHECK-NEXT: {
47+
// CHECK-NEXT: "range": {
48+
// CHECK-NEXT: "end": {
49+
// CHECK-NEXT: "character": 12,
50+
// CHECK-NEXT: "line": 0
51+
// CHECK-NEXT: },
52+
// CHECK-NEXT: "start": {
53+
// CHECK-NEXT: "character": 7,
54+
// CHECK-NEXT: "line": 0
55+
// CHECK-NEXT: }
56+
// CHECK-NEXT: },
57+
// CHECK-NEXT: "uri": "file:///{{.*}}/include/ifdef.sv"
58+
// CHECK-NEXT: }
59+
// CHECK-NEXT: ]
60+
// CHECK-NEXT:}
61+
// -----
62+
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
63+
// -----
64+
{"jsonrpc":"2.0","method":"exit"}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
include/ifdef.sv
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module ifdef(output out);
2+
assign out = 1'b1;
3+
endmodule

0 commit comments

Comments
 (0)