[PATCH] D150370: Introduce StructuredData

[Sebastian Neubauer via Phabricator via llvm-commits]

Flakebi added a comment.

> What kind of tests do you have in mind?

I thought about tests for the parsing and printing code. I missed that the code is not yet used here, so please ignore my comment from before :)

Having global state makes things easy at the start (and efficient in the case here), but I fear we will run into problems later, for example when unloading and reloading libLLVM. There is a precedent for globals, command line arguments are global state and it is problematic.

The `TargetTypeInfoDeserialize::registerSymbols()` call is in the LLVMContext constructor, so it does not seem far-fetched to me to make symbols part of the context.



================
Comment at: llvm/docs/LangRef.rst:3519-3523
+    sdata       ::= '{' (sdata_field ',')* sdata_field? '}'
+    sdata_field ::= label sdata_value
+    sdata_value ::= 'type' type
+                ::= integer
+                ::= 'true' | 'false'
----------------
nhaehnle wrote:
> Flakebi wrote:
> > Can constants follow the way metadata (and most other things in LLVM) is encoded and prefix the type?
> > I.e. `i32 <integer>` and `i1 true/false`
> Good question. I've been going back and forth on this, and the current version is as-is mostly because the various !DIxyz metadata doesn't have those prefixes. But that may not be the best example to follow.
> 
> I'd be happy to change it to be more in line with metadata. Any other opinions?
> But that may not be the best example to follow.

I once tried to write a parser for !DI metadata (tree-sitter, for syntax highlighting) and gave up because there were more and more edge-cases, so I’d be happy to see a more structured format ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150370/new/

https://reviews.llvm.org/D150370