Currently, the extension directly uses CompilationState (ast, files, diagnostics) from slicec.
This PR adds a new type to replace it: CompilationData. This type only holds (ast, files).
It does this to simplify how we get, and publish diagnostics.
Currently:
We compile immediately. The moment we create a new ConfigurationSet, it compiles.
Usually, we aren't ready to publish diagnostics at this point, so we need to temporarily store them, just for a bit.
CompilationState has a required diagnostics field. When we're ready to publish diagnostics, we use mem::take to
take the diagnostics from the state and publish them. This means we have to be careful to only call this after freshly compiling (otherwise there's nothing to take), and make sure we don't touch diagnostics afterwards (since it was taken).
So, it being a required field is a little strange. 99% of the time it's actually been taken out.
I also think it's a little strange that just making a Configuration triggers a compilation.
That's pretty heavy for a constructor, and I'd expect it to be a different function you call on it.
CompilationData solves these problems:
You can create it, without triggering a compilation.
It has a separate function for that: trigger_compilation.
This function returns you the diagnostics.
So, now we create the CompilationData, then, when we're actually ready to compile and report diagnostics,
we call trigger_compilation. No more mem::take, no more hackily storing the diagnostics temporarily.
It also has the side-benefit of centralizing where we compile.
Before this PR, we call slicec::compile_from... in 3 places. Now it's only called from trigger_compilation.
Currently, the extension directly uses
CompilationState(ast, files, diagnostics) fromslicec. This PR adds a new type to replace it:CompilationData. This type only holds (ast, files). It does this to simplify how we get, and publish diagnostics.Currently:
ConfigurationSet, it compiles. Usually, we aren't ready to publish diagnostics at this point, so we need to temporarily store them, just for a bit.CompilationStatehas a requireddiagnosticsfield. When we're ready to publish diagnostics, we usemem::taketo take the diagnostics from the state and publish them. This means we have to be careful to only call this after freshly compiling (otherwise there's nothing to take), and make sure we don't touchdiagnosticsafterwards (since it was taken). So, it being a required field is a little strange. 99% of the time it's actually been taken out.I also think it's a little strange that just making a
Configurationtriggers a compilation. That's pretty heavy for a constructor, and I'd expect it to be a different function you call on it.CompilationDatasolves these problems:trigger_compilation.So, now we create the
CompilationData, then, when we're actually ready to compile and report diagnostics, we calltrigger_compilation. No moremem::take, no more hackily storing the diagnostics temporarily.It also has the side-benefit of centralizing where we compile. Before this PR, we call
slicec::compile_from...in 3 places. Now it's only called fromtrigger_compilation.