[PATCH] D112865: [mlir][core] Inline Intrinsic Attributes

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar added a comment.
Herald added a subscriber: sdasgup3.

Could you expand the description to give the why's of the change?



================
Comment at: mlir/include/mlir/IR/OperationSupport.h:532
+namespace detail {
+class OpAttributeList {
+  /// Use SmallVector with zero inlined elements. SmallVector should use
----------------
Class comment?


================
Comment at: mlir/include/mlir/IR/OperationSupport.h:533
+class OpAttributeList {
+  /// Use SmallVector with zero inlined elements. SmallVector should use
+  /// `unsigned` as its size type.
----------------
Not sure the unsigned comment make sense. Many also consider using unsigned to represent non-negative constraint as anti-pattern (see previous discussion on list).


================
Comment at: mlir/include/mlir/IR/OperationSupport.h:567
+
+  /// Delete an attribute, if one was found. Returns the erased value.
+  template <typename NameT>
----------------
Also document what is returned when it is not found.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112865