[PATCH] D72292: [mlir] NFC: Move the state for managing aliases out of ModuleState and into a new class AliasState.

[Lei Zhang via Phabricator via llvm-commits]

antiagainst accepted this revision.
antiagainst added a comment.
This revision is now accepted and ready to land.

Nice! I assume this is mostly refactoring so NFC.



================
Comment at: mlir/lib/IR/AsmPrinter.cpp:171
+  /// Return a name used for an attribute alias, or empty if there is no alias.
+  Twine getAttributeAlias(Attribute attr) const;
 
----------------
Using Twine is kinda dangerous. Do we really need it here?


================
Comment at: mlir/lib/IR/AsmPrinter.cpp:317
+  for (auto &kindAlias : attrKindToAlias) {
+    for (unsigned i = 0, e = kindAlias.second.second.size(); i != e; ++i)
+      printAlias(kindAlias.second.first, kindAlias.second.second[i], i);
----------------
Nit: it would be slightly better to use a local variable for `kindAlias.second`.


================
Comment at: mlir/lib/IR/AsmPrinter.cpp:427
+  /// null if one wasn't registered.
+  const OpAsmDialectInterface *getOpAsmInterface(Dialect *dialect) {
+    return interfaces.getInterfaceFor(dialect);
----------------
Nit: Why inling this one but not the above one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72292