[mlir] fix bugs with NamedAttrList

[Mogball]

Summary

• assign with ArrayRef was calling append
• assign with empty ArrayRef was not clearing storage

[jpienaar]

Thanks for fixing my typo :-)

mlir/lib/IR/BuiltinAttributes.cpp

72

Could we use clear instead?

mlir/unittests/IR/OperationSupportTest.cpp

245

Do you want to check contents as above to ensure this was an assign?

[Mogball]

Rebase with upstream

mlir/include/mlir/IR/OperationSupport.h

void append(IteratorT in_start, IteratorT in_end) {
     attrs.append(in_start, in_end);
   }
 
   /// Replaces the attributes with new list of attributes.
   void assign(const_iterator in_start, const_iterator in_end);
 
   /// Replaces the attributes with new list of attributes.
   void assign(ArrayRef<NamedAttribute> range) {
-    append(range.begin(), range.end());
+    assign(range.begin(), range.end());
   }
 
   bool empty() const { return attrs.empty(); }
 
   void reserve(size_type N) { attrs.reserve(N); }
 
   /// Add an attribute with the specified name.
   void push_back(NamedAttribute newAttribute);

mlir/lib/IR/BuiltinAttributes.cpp

 /// whether source was sorted.
 template <bool inPlace>
 static bool dictionaryAttrSort(ArrayRef<NamedAttribute> value,
                                SmallVectorImpl<NamedAttribute> &storage) {
   // Specialize for the common case.
   switch (value.size()) {
   case 0:
     // Zero already sorted.
+    if (!inPlace)
+      storage.clear();
     break;
   case 1:
     // One already sorted but may need to be copied.
     if (!inPlace)
       storage.assign({value[0]});
     break;
   case 2: {
     bool isSorted = value[0] < value[1];

mlir/unittests/IR/OperationSupportTest.cpp

TEST(OperationFormatPrintTest, CanUseVariadicFormat) {
   Builder builder(&context);
 
   Operation *op = createOp(&context);
 
   std::string str = formatv("{0}", *op).str();
   ASSERT_STREQ(str.c_str(), "\"foo.bar\"() : () -> ()");
 }
 
+TEST(NamedAttrListTest, TestAppendAssign) {
+  MLIRContext ctx;
+  NamedAttrList attrs;
+  Builder b(&ctx);
+
+  attrs.append("foo", b.getStringAttr("bar"));
+  attrs.append("baz", b.getStringAttr("boo"));
+
+  {
+    auto it = attrs.begin();
+    EXPECT_EQ(it->first, b.getIdentifier("foo"));
+    EXPECT_EQ(it->second, b.getStringAttr("bar"));
+    ++it;
+    EXPECT_EQ(it->first, b.getIdentifier("baz"));
+    EXPECT_EQ(it->second, b.getStringAttr("boo"));
+  }
+
+  attrs.append("foo", b.getStringAttr("zoo"));
+  {
+    auto dup = attrs.findDuplicate();
+    ASSERT_TRUE(dup.hasValue());
+  }
+
+  SmallVector<NamedAttribute> newAttrs = {
+      b.getNamedAttr("foo", b.getStringAttr("f")),
+      b.getNamedAttr("zoo", b.getStringAttr("z")),
+  };
+  attrs.assign(newAttrs);
+
+  auto dup = attrs.findDuplicate();
+  ASSERT_FALSE(dup.hasValue());
+
+  {
+    auto it = attrs.begin();
+    EXPECT_EQ(it->first, b.getIdentifier("foo"));
+    EXPECT_EQ(it->second, b.getStringAttr("f"));
+    ++it;
+    EXPECT_EQ(it->first, b.getIdentifier("zoo"));
+    EXPECT_EQ(it->second, b.getStringAttr("z"));
+  }
+
+  attrs.assign({});
+  ASSERT_TRUE(attrs.empty());
+}
 } // end namespace