[PATCH] D29610: Fix BucketT Handling in DenseMap.

[Dean Michael Berris via Phabricator via llvm-commits]

dberris requested changes to this revision.
dberris added a comment.
This revision now requires changes to proceed.

You need to add unit tests to show what the bug is, and how this change is fixing it. That should be added as a test case in the existing unit test for DenseMap. Right now I'm inclined to reject this change given the issue raised inline.



================
Comment at: include/llvm/ADT/DenseMap.h:111
+          P->~BucketT();
+          ::new (&P->getFirst()) KeyT(EmptyKey);
           --NumEntries;
----------------
This smells funny -- first you destroy the object, then call a function on the destroyed object. I'm positive you are running into undefined behaviour here. To fix this, you should attempt to re-initialize the object pointed to through P using placement new.


https://reviews.llvm.org/D29610