[llvm] [llvm][Support] Add YAMLGenerateSchema for producing YAML schemas (PR #133284)

[Timur Golubovich via llvm-commits]

tgs-sc wrote:

Hi @slinder1!

First, thanks for the insightful comments, this is a big rarity in this review.

> @tgs-sc Did you generate any of this code using an LLM or consult an LLM to make some of the design choices?

Well, when I started working on this patch LLMs' were not able to help me with this patch (I don't know whether this is good or bad as you asked :) ).

> I am just seeing a unique mix of things which (to me) imply both high-level understanding of a complicated (in terms of language features used/abused) part of the codebase as well as fundamental misuse of some C++ facilities.

I know that this patch can look a little bit magic, as for now c++ doesn't support reflexivity and parsing YAML or other formats - require requires some additional effort  from the programmer. But as current implementation can be extended for generating schemas, why don't to do this? Honestly, as I understood, your main complaint is about the vector<unique_ptr>. It just seemed to me that using an allocator here wouldn't significantly improve performance. Likely I'm wrong, but overall, it's not that big of a problem.

> I don't want to invest more effort in the review until I understand better how I can approach it in a way that benefits you, me, and the llvm-project.

Well, to be fair the real help would be to provide an opinion if you think that the idea of patch is beneficial for llvm project. Currently, I'm struggling to find reviewers have are committed to provide a definite statement if this is worth merging or not. I have an impression (I may be wrong here, though) that yaml-based support is not in active developement so not that many people can provide a constructive feedback.

> I am including the comments I had already drafted by the time I decided to stop and ask

Thank you, I appreciate it. I will address all these. But can you please tell me if you will approve it after I fix everything, i.e., in general, are you ready to come to terms with the idea of ​​this patch?

https://github.com/llvm/llvm-project/pull/133284