[PATCH] D78844: Sparse String Elements support.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/IR/Attributes.h:1255
+  /// Gets a zero (empty) stringref for the given sparse attribute.
+  StringRef getZeroString() const { return StringRef(); };
+
----------------
You can just inline this in the `getZeroValue<StringRef>` below. The other methods are only defined like this because the values can't be constructed inline in the header file.


================
Comment at: mlir/include/mlir/IR/Attributes.h:1282
+  /// Get a zero for an APFloat.
+  StringRef getZeroValue() const { return getZeroString(); }
   /// Get a zero for an C++ integer or float type.
----------------
This needs to be a SFINAE template to avoid having ambiguous methods.


================
Comment at: mlir/include/mlir/IR/Attributes.h:1283
+  StringRef getZeroValue() const { return getZeroString(); }
   /// Get a zero for an C++ integer or float type.
   template <typename T>
----------------
nit: Newline before here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78844