[PATCH] D44109: TableGen: Add a defset statement

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added inline comments.


================
Comment at: docs/TableGen/LangRef.rst:360-368
+``defset``
+----------
+.. productionlist::
+   Defset: "defset" `Type` `TokIdentifier` "=" "{" `Object`* "}"
+
+All records inside the braces are defined normally, and are additionally
+collected in a globally accessible list of the given name. The given type
----------------
tra wrote:
> Does it mean that instantiating any record that is not of type A will be an error? That sounds rather limiting.
> That would prevent use of anonymous records or non-anonymous records or different types we may want to instantiate within the same multiclass hierarchy without having to duplicate all of it.
> 
> Could we change the semantics to `the list collects all instantiated records of type A` and, maybe, allow more than one pair of type:list , so we can collect multiple record types?
> 
> Another approach would be to say -- give me all records of type A, created by this defm instance. I.e. instead of `defset A foo = { records }` you do something like `defm FOO : M<...>; list<A> foo = !defset(FOO)`. 
> 
Anonymous (or rather "inline"?) records are not an issue: they are created on a different path and never added to a defset, and I think they shouldn't be. I've clarified that.

I did make incompatible (non-anonymous) record types an error because one could accidentally make the type of `list<A>` too strict and then end up with records silently not being collected in surprising ways.

I see your point about allowing different types explicitly, but I'm not entirely convinced yet. In part the whole point of `defset` is that it helps avoid reducing duplication by using the resulting list in a `foreach` later. I'm also a bit hesitant about proliferation of `defset`, and I'd personally err on the side of starting with stricter semantics. It's always easier to go to looser semantics later if good use cases for it show up than the other way around.


Repository:
  rL LLVM

https://reviews.llvm.org/D44109