[PATCH] [ADT][CMake][AutoConf] fail-fast iterators for DenseMap

[David Blaikie]

On Mon, Mar 16, 2015 at 9:03 AM, Zachary Turner <zturner at google.com> wrote:

> If I understand correctly, this broke before because LLDB would redefine
> _NDEBUG sometimes, but not other times when including clang headers.
>
> Now, since this isn't based on _NDEBUG, this is no longer an issue. If so,
> looks good to me.


Ah, right right - thanks for explaining/reminding me.


>
> On Mon, Mar 16, 2015 at 8:35 AM David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Sun, Mar 15, 2015 at 10:48 PM, Sanjoy Das <
>> sanjoy at playingwithpointers.com> wrote:
>>
>>> Hi chandlerc, dexonsmith, rnk, zturner,
>>>
>>> This is a re-do of D7931 with the following changes:
>>>
>>>  * the `EpochTracker` class is now conditional on
>>>    `LLVM_ENABLE_FAIL_FAST_ITERATORS` and not `NDEBUG`.
>>>
>>
>> It'd be nice to have a more general conditional name so we can add other
>> ABI-breaking debug checks under the same flag. (strawman:
>> LLVM_ABI_BREAKING_DEBUG ?)
>>
>>
>>>
>>>  * **The most significant change**: `LLVM_ENABLE_FAIL_FAST_ITERATORS`
>>>    can be forced to be set or unset independent of `NDEBUG`.  By
>>>    default, cmake and autoconf set it (thus enabling fail-fast
>>>    iterators) for with-asserts builds and unset it for without-asserts
>>>    builds.
>>>
>>
>> *fingers crossed that this is sufficient for other users* (that they're
>> willing to manually unset this value for their debug builds if they need
>> ABI compatibility)
>>
>>
>>>
>>> With these changes, projects that require ABI compatibility between
>>> `defined(NDEBUG)` builds and `!defined(NDEBUG)` builds can get that by
>>> clamping `LLVM_ENABLE_FAIL_FAST_ITERATORS` to on or off.
>>>
>>> == Minor changes from D7931 ==
>>>
>>>  * `HandleBase::getEpochAddress` returns a `void *` to emphasise that
>>>    the result is for comparison, not derefernecing.
>>>
>>>  * `HandleBase::getEpochAddress` was missing in the fail-fast-disabled
>>>    version of `HandleBase`.  This was not a problem earlier since the
>>>    function is only called from `assert`s and when `assert`s were
>>>    enabled, we'd always choose the fail-fast-enabled version of
>>>    `HandleBase`.  Since this is no longer the case (a with-asserts
>>>    build may be using the fail-fast-disabled version of `HandleBase`),
>>>    the missing method shows up as an obvious compiler error.
>>>
>>> Original summary for D7931:
>>> This patch is an attempt at making `DenseMapIterator`s "fail-fast".
>>> Fail-fast iterators that have been invalidated due to insertion into
>>> the host `DenseMap` deterministically trip an assert (in debug mode)
>>> on access, instead of non-deterministically hitting memory corruption
>>> issues.
>>>
>>> http://reviews.llvm.org/D8351
>>>
>>> Files:
>>>   CMakeLists.txt
>>>   Makefile.config.in
>>>   Makefile.rules
>>>   autoconf/configure.ac
>>>   cmake/modules/HandleLLVMOptions.cmake
>>>   configure
>>>   docs/CMake.rst
>>>   include/llvm/ADT/DenseMap.h
>>>   include/llvm/ADT/EpochTracker.h
>>>
>>> EMAIL PREFERENCES
>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150316/087fd3e3/attachment.html>