[PATCH] D41951: [Attributes] Fix crash when attempting to remove alignment from an attribute list/set
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 12 08:58:13 PST 2018
reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.
A few minor changes needed, but generally looks close to ready.
================
Comment at: lib/IR/Attributes.cpp:564
// For now, say we can't pass in alignment, which no current use does.
assert(!Attrs.hasAlignmentAttr() && "Attempt to change alignment!");
----------------
Incomplete handling. I think the assert is simply stale and missing a test case.
================
Comment at: lib/IR/Attributes.cpp:1101
if (!hasAttribute(Index, Kind)) return *this;
- AttrBuilder B;
- B.addAttribute(Kind);
- return removeAttributes(C, Index, B);
+ if (!pImpl)
+ return AttributeList();
----------------
I believe this check is implied by the hasAttribute check above it.
================
Comment at: unittests/IR/AttributesTest.cpp:81
+ EXPECT_FALSE(AL.hasParamAttr(0, Attribute::Alignment));
+}
+
----------------
Add a test for remove AttrBuilder case.
Also add a test case for a non-param alignment or at least one of them. Say, stackalign for the AttrBuilder case.
https://reviews.llvm.org/D41951
More information about the llvm-commits
mailing list