How to update when RTL changes

[ichigensanokotowari]

Sorry, I have a super basic question, probably caused by me just not using things in the right way. Basically, I am trying to use slang with my RTL editor to do syntax highlighting and error detection. I have one global SourceManager (that uses getDefaultSourceManager()), and I add files via
slang::SourceBuffer buffer = sm.assignText(path, code, loc);

I can then make a SyntaxTree with SyntaxTree::fromBuffer(), get the compilation with Compilation::addSyntaxTree(), and then get errors with Compilation::getAllDiagnostics()

All of that works. However, I am not sure what needs to get called when a source file changes. I'm not immediately seeing a way to remove/readd a buffer to the source manager, and I don't immediately see a way to update the text in an existing buffer.

I guess the larger more general questions are

0. am I even using things correctly, or is it meant to be used in a different way?

1. if I want to update errors and syntax highlighting in real time, what needs to be done when an existing RTL file changes?

2. if I add a file to SourceManager and get a buffer back, does SourceManager need to know when a registered file is deleted/changed

3. am I meant to have one syntax tree per buffer? Would it know about definitions of things in other files? Does it need to be destroyed/rebuilt when a file changes?

So sorry if these are basic questions, but I don't really see any documentation on how to use things so I am just taking random guesses based on useful-sounding function names I see

[MikePopoloski]

0. Yes, it seems like you're using it correctly.

1 & 2: Text loaded into the SourceManager can't be safely unloaded because everything downstream from it (syntax trees, AST symbols, etc) contain pointers into that text instead of copying. When the source changes you need to load a new buffer and reparse it, and then rebuild the AST from the new buffer. To unload the old text, the only way currently is to destroy the SourceManager and make a new one. This is obviously wasteful if you have a bunch of buffers loaded into memory and only want to change one of them, so it would make sense to me to add a method to migrate buffer memory between SourceManagers so you could recreate quickly with all of the files that did not change and then just load the one that did. Such a method does not exist currently but it would not be too hard to add I think, but you would need to ensure that there are no existing syntax or AST objects that reference the old buffer memory.

3. This depends on the project. SystemVerilog has two compilation modes: one where each file is a separate "compilation unit" like you would expect in other languages, and one where all of the files get concatenated together into a single compilation unit. The major difference between the two is that in the second method the preprocessor / macro definitions are visible in subsequent files, whereas in the first method they are scoped to their own file only. Symbols likes module definitions and packages are visible across compilation units so it doesn't matter which mode you use, you're still able to e.g. instantiate modules from other files. I personally think the first method is the only one that projects should use but plenty of real world projects use the second so slang supports both.

If you're using the first method, then you create a separate SyntaxTree per buffer and only need to reparse (i.e. recreate a new SyntaxTree) for a single file whenever it changes. If you're using the second method then you create a single SyntaxTree from all of the buffers (using fromBuffers) and need to reparse everything whenever a file changes (because you don't know how parsing later files will change based on macro definitions that happen inside the changed file).

[ichigensanokotowari]

Thanks, Mike. So just to confirm, if I want do something where you right click on a symbol in one file to jump to a definition in another file, they all need to be part of the same syntax tree? And I can keep unmodified buffers around, but when anything changes, I need to take all the buffers and rebuild the syntax tree?

additional questions that I suspect I may know the answer to:

0. is there any way to combine syntax trees? So if I had 16 files, 4 syntax trees, and 4 files per tree, if any one file changed I could rebuild only it's syntax tree, and then somehow magically combine it with the other three unmodified trees? Just as a way to mitigate some of the cost. Disclaimer: I don't know how heavy of an operation parsing and building trees is, so maybe it's not that bad?

1. I worry about the source manager growing out of control. In the case where I don't do anything smart, technically every editor keystroke is a change in text, and therefore I would have to create a new buffer for every keystroke event while keeping the old buffers around. Just want to make sure, but it sounds like there isn't much I can do to get around this aside from maybe slightly reducing updates by only rebuilding during pauses after typing stops. But it still feels like the size can get out of control very fast after an hour of RTL coding

[MikePopoloski]

No, it's not true that you need one syntax tree in order for jump to definition to work. At the symbol level (i.e. the "AST") symbols can exist in separate compile units (separate syntax trees) and still refer to each other. If you have such a symbol you can look at its SourceLocation and from there know where the symbol was typed in the source.

0. You can't combine syntax trees after they've already been created. It's possible to add this feature in a kind of superficial way by just gluing two CompilationUnit syntax nodes together from each tree, but what you really want is to combine the source files and there's simply no way that could ever work, because you might define macros in one file and have them used in the second file in a way that totally changes the interpretation of the source. There's simply no way to know in a general fashion how that might change the parsed nodes, so you always have to reparse. As I mentioned, this is why projects should prefer the multi-compilation unit model instead of the single-unit model, because then you can parse each source file in isolation. (As a side note, the slang parser is pretty fast so you might not have a problem in practice, but if your users are using the single-unit model and has a ton of source files then basically no parser out there is going to be able to do a good job).

1. Yes, that's definitely a concern and that's why I was suggesting adding a feature to SourceManager to let you unload a specific buffer or copy unmodified buffers over to a new SourceManager or something along those lines. You just have to be very careful about using such a feature because if you unload a buffer's memory you have to be very sure you have no AST nodes or SyntaxTrees around still pointing to that memory.

[suzizecat]

Hi there !

So, I'm currently trying to do more or less the same thing as the OP (in my case a Language Server, LS) and would like to minimize file re-parsing. After reading this discussion, I understand that, in order to reload a single file you need to delete the SourceManager which is associated to it and recreate it. Therefore, I understand that I shall have one source manager per file.

Now, as I want to be able to performs actions on the elaborated design (like getting diagnostics, references and so on), I need to compile the design.
In my LS implementation, I have made the following function, which role is to re-run the compilation on the whole design:

void DiplomatLSP::_compile()
{
    spdlog::info("Request design compilation");
    // Create a structure which holds a source manager and a syntax tree per-file.
    _read_workspace_modules();

    // Reset the internal compilation object
    _compilation.reset(new slang::ast::Compilation());

    //Prepare the diagnostics
    slang::DiagnosticEngine de(*(_compilation->getSourceManager()));
    de.addClient(std::shared_ptr<DiplomatLSP>(this)); // NB : This is probably quite bad
    

    spdlog::info("Add syntax trees");
    for (const auto& [key, value] : _documents)
    {
        _compilation->addSyntaxTree(value->st);
    }

    // Run the compilation
    _compilation->getRoot();

    spdlog::info("Issuing diagnostics");
    for (const slang::Diagnostic& diag : _compilation->getAllDiagnostics())
        de.issue(diag);
}

This version is a bit simple (I actually re-real all sources) but the idea is here. Each element stored in _documents is a structure with a syntax tree and its associated SourceManager.

Now, when I add the syntax trees to the compilation through _compilation->addSyntaxTree(value->st);, I get the following error (at runtime):

terminate called after throwing an instance of 'std::logic_error'
  what():  All syntax trees added to the compilation must use the same source manager

Which seems reasonable to me. But then my question, should I have used the same SourceManager when creating all syntax trees ? If so, wouldn't all the syntax trees be invalidated when I delete the SourceManager ?

[MikePopoloski]

Yes, the requirement that all syntax trees in a compilation use the SourceManager is not really avoidable. Like I was saying earlier in the thread, I think the right approach is to add a feature to SourceManager to allow reloading a buffer -- it's pretty easy to do, the only problem is that you as an API user need to be very careful when doing a reload because tokens, syntax nodes, AST nodes, the Compilation object, source locations, etc all point into the buffer (typically via a simple string_view) and reloading the buffer essentially invalidates all of them. There's no way for me to make the API safer to use without severely compromising compilation performance, so it'll be up to you to use it correctly.

[suzizecat]

I indeed approve of your remarks and understand the various issues at hand here. This feature comes with various risks.

As a side note, I just found out that ,as it is not currently possible to update the source manager properly, it is also required to recreate any DiagnosticEngine (that const-reference the source manager) each time the source manager is recreated. Which, in turn, invalidate all diagnostic clients.

[MikePopoloski]

I took a look at this and it's even a bit more complicated still; each file is only loaded into memory once but may be referred to via different BufferIDs -- this is because it may be included from multiple locations (each of which can interpret the file differently due to different macros being defined at the point of include) and also because each macro expansion creates a new BufferID which ultimately links back to the original file.

This means a few things; first, your request for a getBufferID() from a given path might have to return a range of buffers, which may or may not be what you want. Second, it means that when you reload() a file you can't necessarily reclaim other BufferIDs that were pointing to it, so eventually the BufferID space will continue to grow and run out of memory or just run out of ID space to assign. I'm not entirely sure the best way to handle this. If you have thoughts feel free to share.

[suzizecat]

Hmmm... I do not quite understand why would you need multiple buffers for one file. Are the files stored after pre-processing ?
I thought that the buffer was the "raw" representation of the file. For the topic of claiming the BufferIDs, I would probably use some kind of chained list or queue in which you could add the invalidated BufferIDs and then reuse them instead of a new buffer if possible.
The algorithm to get a new buffer ID would then be something around the lines of:

if (_invalidatedBuffers.size() > 0)
    my_new_bufId = _invalidatedBuffers.pop();
else
    my_new_bufId = _last_bufId +1 ;

This way, you wouldn't need to create an actual object for the bufferID nor change the buffer object.

On the same theme, I would expect the syntax tree to be agnostic of the defines, macros and such and to be "resolved" at elaboration time for symbols computation.

[MikePopoloski]

Well you need a way to refer back to each expansion of a macro, both pointing to its original location as well as its expansion location, which necessarily requires pointing to a buffer somewhere. Reloading the file behind such a buffer would require knowing all of the macros that were expanded from it so you can recycle the IDs. Doable certainly, but it's definitely a decent amount of work.