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

[Sanjoy Das]

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 ?)

That's a great idea -- I'll
s/LLVM_ENABLE_FAIL_FAST_ITERATORS/LLVM_ENABLE_ABI_BREAKING_ASSERTS/

-- Sanjoy

>
>>
>>
>>  * **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
>>
>