[PATCH] D73362: [mlir] EnumsGen: dissociate string form of integer enum from C++ symbol name

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added a comment.



> I'm not sure I quite understand. I did add an additional parameter to `EnumAttrCaseInfo` and it is defaulted to C++ enumerant symbol for `StrEnumAttrCase` and for `I32EnumAttrCase`/`I64EnumAttrCase` that are currently in use. It made more sense to me to have this value at the top-level class exactly because it could be useful for other kinds of attributes. I could not have `IntEnumAttrCaseBase` that inherits from `IntStrEnumAttrCaseBase` for the purposes of defaulting the string representation to C++ enumerant since, in this case, I32EnumAttr and I64EnumAttr would have to be duplicated instead. They accept I32/64StrEnumAttrCase so can also accept the derived I32/64EnumAttrCase now, but it would not be correct if the defaulting happened before the type is specified.

I meant that instead of having both `I32EnumAttrCase` (taking two parameters) and `I32StrEnumAttrCase` (taking three parameters), what about just having `I32EnumAttrCase` and making it taking in `string cSym, int cVal, string strVal = cSym`? Similarly for other `*EnumAttrCase`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73362