[Mlir-commits] [mlir] [mlir][ODS] Add namespace filtering to `-gen-enum-*` (PR #89627)

[Markus Böck]

zero9178 wrote:

> > ]. As soon as we then include the Enums.td in these it is also transitively included in all other dialects using that dialects attributes, types or ops.
> 
> Yes, but that still does not explain why it is a problem :)
> 
> With a `Enums.td` file: each dialect will generate their enum without seeing the definition of the other enums: because the transitive inclusion you're describing does not apply to enums...

I see your point yes. As in, `DialectBEnums.td` that is conceptually part of `DialectB` that depends on `DialectA` will not see `DialectAEnums.td` since there is no reason for that file to ever include any TableGen file from DialectA, right?

> Maybe, but then this is what you're trying to solve and the PR should documenting as such. Right now your first paragraph make it sounds like more than it is: if there was such a fundamental problem, how could all of the existing enums work?

I don't mind changing the description. At least upstream it hasn't been that big of an issue as most upstream dialects don't `include` the TD files of another dialect. Defining enums in anything but a dedicated `*Enums.td` is sadly not uncommon however (`git grep EnumAttr` shows many such places). We can view these as being bugs in need of fixing or a need for a convention to be established and documented, but that seems like a larger discussion than this PR.

> * The use of the C++ namespace is a bit... unusual. Dialects are allowed to share a namespace, and the enums as well.

I don't feel too strongly about the mechanism and felt this to be the most natural. While two dialects could share a C++ namespace I think this to be rather unconventional and not something I've seen upstream or downstream.
 
What do you think about having a regex for the record names of the enums instead? Lets say the vector dialect defines all its enums as `Vector_*`, a regex filter of `Vector_.*` would then only generate all enums whose `def` name starts with `Vector_`.

> * The "implicit" regex seem fragile: if I have a mlir::test namespace and another dialect toy::test namespace, the filtering on test is broken.

I am not sure what you mean with broken. As it is currently implemented, specifying `test` would not generate enums for either of these two namespaces by design since they're not subnamespaces of `test` (rather of `mlir::test` and `toy::test` respectively). 

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