[PATCH] D41951: [Attributes] Fix crash when attempting to remove alignment from an attribute list/set

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/IR/Attributes.cpp:1106
+  SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
+  if (Index >= AttrSets.size())
+    AttrSets.resize(Index + 1);
----------------
I think we can just `assert(Index < AttrSets.size())` because otherwise hasAttribute would return false.


================
Comment at: lib/IR/Attributes.cpp:1109-1111
+  AttrBuilder B(AttrSets[Index]);
+  B.removeAttribute(Kind);
+  AttrSets[Index] = AttributeSet::get(C, B);
----------------
Can you simplify this to `AttrSets[Index] = AttrSets[Index].removeAttribute(Kind);`? It seems you fixed up `AttributeSet::removeAttribute` to handle this case.


================
Comment at: lib/IR/Attributes.cpp:1124
+  SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
+  if (Index >= AttrSets.size())
+    AttrSets.resize(Index + 1);
----------------
ditto, can strengthen to assert.


================
Comment at: lib/IR/Attributes.cpp:1129
+  B.removeAttribute(Kind);
+  AttrSets[Index] = AttributeSet::get(C, B);
+
----------------
ditto, can use `AttributeSet::removeAttribute`.


https://reviews.llvm.org/D41951