Immutable Ast usage

[kvark]

gfx-rs Metal backend stores the shader module in order to compile it with different options for different pipeline layouts (see gfx-rs/gfx#1576). It would be great to store Ast instead of re-parsing it each time, and just provide a reference to CompilerOption at the spot where compilation is needed.

How about we remove set_compiler_options at all and make the Ast::compile(&self, &CompilerOptions) (notice &self)?

[grovesNL]

I would like this too, the challenge is deciding how closely we align with the C++ API. This suggestion matches the original API but changed in #10 to align closer with the SPIRV-Cross classes/methods.

The idea was that you could perform a single parse (into Ast, which has members for all of the Ast data populated during parse, like entry points), then later compile the Ast multiple times, each compile using its own CompilerOptions and even different Target if desired. CompilerOptions would contain every mutation you'd like to perform (i.e. all the set_ functions that currently exist).

We could hide the fact that compile mutates the options internally and be careful to always override the previous state. Although if we went that route it would seem best to move all the other set_ functions into CompilerOptions too for consistency. Likewise we could change the getters back to members on the Ast struct and populate this upfront during parse.

I was hoping to have a copy constructor for the SPIRV-Cross compiler instance. So internally the flow would work like:

• user calls parse
  • C++: create a compiler instance (just for parsing, so the Target doesn't even have to be known)
  • C++: call parse
  • C++: extract the ast from the compiler instance
  • C++/Rust: copy any meta information into public members on Ast (replacing the get_ functions)
  • C++: delete the compiler
  • Rust: return Ast
• user calls compile on Ast
  • C++: create a new compiler instance by copying the existing ast into the instance
  • C++: call all the various setters on the compiler (set_compile_options etc.)
  • C++: call compile and release the returned string
  • C++: delete the compiler

[kvark]

the challenge is deciding how closely we align with the C++ API

In general, since this is not a xxx-sys crate, we should be free to derail from the original API a bit.

then later compile the Ast multiple times, each compile using its own CompilerOptions and even different Target if desired

While it sounds ok, this use case is not needed for gfx-rs. We know the Target, we just don't want to provide any overrides before the parser -> MSL is done.

We could hide the fact that compile mutates the options internally and be careful to always override the previous state.

Yeah, it's an option, just not a pretty one :)

[grovesNL]

While it sounds ok, this use case is not needed for gfx-rs. We know the Target, we just don't want to provide any overrides before the parser -> MSL is done.

Agreed, I just mention it here because it's similar conceptually - setting any options will mutate the internal compiler, so we have to fake immutability by creating new compilers or resetting stale state.

Another idea is to split the API into two levels, where the higher level API supports the flow parse/multiple compile flow above and manages resetting internal compiler state, but the lower level API is mostly 1:1 with the C++ API.

[grovesNL]

I think we might be able to do this now, we'll need to play around with this idea more. As far as I know the parse and compile step have been split apart, so we should be able to reuse the parsed SPIR-V.