Replacing PropertySetIO with YAML #3190
AlexeySachkov
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
There is a TODO item in
PropertySetIO
header:llvm/llvm/include/llvm/Support/PropertySetIO.h
Line 9 in f126512
I'm starting this thread to see our options of resolving this TODO (which includes simply removing it).
I've explored YAML path a little bit: LLVM has support for it (docs) and I guess re-using existing component is better than inventing a new one, but there a few downsides that seem to be significant to me:
Serialization/deserialization to/from YAML is done through user-provided type traits that show how user-defined data structure should be mapped to YAML. This means that we won't be able to just remove
PropertySetIO
- most likely we will have to describe that mapping for our properties, i.e. there still will be our own code which we have to support.Even though YAML file is quite flexible in terms of various data structures that can be represented there, LLVM's YAML engine requires from us to know the exact document structure as it returns instance of particular object as a result of deserialization: something like
std::map
orstd::vector
. This implies two things:a. we will have to establish some fixed structure of the whole properties file, which is opposite of what we are doing right now: our current data structure is flexible and able to handle any amount of property categories of any type
b. that established data structure must be shared across all users of it, i.e. between
sycl-post-link
andclang-offload-wrapper
tools, but the same is true for existingPropertySetIO
To me (a) seems the main problem: our existing property categories are optional and not all of them might be present in all flows. Which top-level structure should be used to describe such YAML file is also unclear: we can't just use mapping, because it would be equivalent to
std::map
and different property categories have properties of different types within them.Possible solutions which I see here is to either use separate file for each property category (doesn't look right) or encoding several YAML documents into a single file, but in that case we probably need to establish some fixed order of their appearance within a file to correctly read them; plus YAML reader and writer have to be synchronized, which complicates adding new properties.
Tagging @kbobrovs and @keryell (commented on #1357) to initiate some discussion
Beta Was this translation helpful? Give feedback.
All reactions