[PATCH] D79011: [mlir] Extract DictionaryAttr sort method

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/IR/Attributes.cpp:109
+template <bool inPlace>
+static bool DictionaryAttrSort(ArrayRef<NamedAttribute> value,
+                               SmallVectorImpl<NamedAttribute> &storage) {
----------------
Please use camelCase naming.


================
Comment at: mlir/lib/IR/Attributes.cpp:126
+      } else {
+        storage.push_back(value[1]);
+        storage.push_back(value[0]);
----------------
Can we change this to `storage.append({value[1], value[0]});` to make this a one liner?


================
Comment at: mlir/lib/IR/Attributes.cpp:134
     // Check to see they are sorted already.
-    bool isSorted = true;
-    for (unsigned i = 0, e = value.size() - 1; i != e; ++i) {
-      if (compareNamedAttributes(&value[i], &value[i + 1]) > 0) {
-        isSorted = false;
-        break;
-      }
-    }
+    auto it = std::adjacent_find(value.begin(), value.end(),
+                                 [](NamedAttribute l, NamedAttribute r) {
----------------
Just use llvm::is_sorted here?


================
Comment at: mlir/lib/IR/Attributes.cpp:163
+void DictionaryAttr::sort(SmallVectorImpl<NamedAttribute> &array) {
+  DictionaryAttrSort</*inPlace=*/true>({}, array);
+}
----------------
Why not pass in array twice here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79011