[PATCH] D93780: ADT: Fix reference invalidation in N-element SmallVector::append and insert

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 13 20:25:33 PST 2021


dexonsmith marked an inline comment as done.
dexonsmith added a comment.

Thanks for the review! Landed in 3043e5a5c33c4c871f4a1dfd621a8839f9a1f0b3 <https://reviews.llvm.org/rG3043e5a5c33c4c871f4a1dfd621a8839f9a1f0b3>.



================
Comment at: llvm/unittests/ADT/SmallVectorTest.cpp:1123
+  // use-after-free here.
+  V.append(V.capacity(), V.front());
+  EXPECT_EQ(1, V.back());
----------------
dblaikie wrote:
> Might be worth making this V.capacity() - V.size() + 1 to make it more precise? While V.capacity() will always be enough, I think (to me at least) that might make the test a bit harder to read, since capacity() may be significantly more than needed.
Good point. I updated as suggested in the commit.

In the similar case for the `InsertN` test below, I decided to expand the comment and leave it as `V.capacity()` since being precise there made it harder to reason about (since there we care about `NumToInsert` being larger than something).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93780



More information about the llvm-commits mailing list