<div dir="ltr">As far as I know this is fine, because LLDB always builds the clang that it links against.  It's certainly the case for the Windows and Linux buildbots.  +Greg Clayton in case he knows something different on the apple side.<br><br><div class="gmail_quote">On Mon, Mar 16, 2015 at 10:45 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On 2015 Mar 16, at 09:03, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
><br>
> If I understand correctly, this broke before because LLDB would redefine _NDEBUG sometimes, but not other times when including clang headers.<br>
><br>
<br>
I think LLDB's NDEBUG inconsistency is a separate issue, and should<br>
probably be cleaned up/removed (as I pointed out, the original reason<br>
for redefining NDEBUG was cleaned up years ago).<br>
<br>
If the LLVM/clang libraries that LLDB links against are built with<br>
FAIL_FAST=false, and LLDB builds with FAIL_FAST=true, then you're<br>
still going to have an ABI problem.<br>
<br>
IIUC, with this patch LLDB would need to match FAIL_FAST with the<br>
LLVM/clang libraries it's linking against.<br>
<br>
> Now, since this isn't based on _NDEBUG, this is no longer an issue. If so, looks good to me.<br>
> On Mon, Mar 16, 2015 at 8:35 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> On Sun, Mar 15, 2015 at 10:48 PM, Sanjoy Das <<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.<u></u>com</a>> wrote:<br>
> Hi chandlerc, dexonsmith, rnk, zturner,<br>
><br>
> This is a re-do of D7931 with the following changes:<br>
><br>
>  * the `EpochTracker` class is now conditional on<br>
>    `LLVM_ENABLE_FAIL_FAST_<u></u>ITERATORS` and not `NDEBUG`.<br>
><br>
> 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 ?)<br>
><br>
><br>
>  * **The most significant change**: `LLVM_ENABLE_FAIL_FAST_<u></u>ITERATORS`<br>
>    can be forced to be set or unset independent of `NDEBUG`.  By<br>
>    default, cmake and autoconf set it (thus enabling fail-fast<br>
>    iterators) for with-asserts builds and unset it for without-asserts<br>
>    builds.<br>
><br>
> *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)<br>
><br>
><br>
> With these changes, projects that require ABI compatibility between<br>
> `defined(NDEBUG)` builds and `!defined(NDEBUG)` builds can get that by<br>
> clamping `LLVM_ENABLE_FAIL_FAST_<u></u>ITERATORS` to on or off.<br>
><br>
> == Minor changes from D7931 ==<br>
><br>
>  * `HandleBase::getEpochAddress` returns a `void *` to emphasise that<br>
>    the result is for comparison, not derefernecing.<br>
><br>
>  * `HandleBase::getEpochAddress` was missing in the fail-fast-disabled<br>
>    version of `HandleBase`.  This was not a problem earlier since the<br>
>    function is only called from `assert`s and when `assert`s were<br>
>    enabled, we'd always choose the fail-fast-enabled version of<br>
>    `HandleBase`.  Since this is no longer the case (a with-asserts<br>
>    build may be using the fail-fast-disabled version of `HandleBase`),<br>
>    the missing method shows up as an obvious compiler error.<br>
><br>
> Original summary for D7931:<br>
> This patch is an attempt at making `DenseMapIterator`s "fail-fast".<br>
> Fail-fast iterators that have been invalidated due to insertion into<br>
> the host `DenseMap` deterministically trip an assert (in debug mode)<br>
> on access, instead of non-deterministically hitting memory corruption<br>
> issues.<br>
><br>
> <a href="http://reviews.llvm.org/D8351" target="_blank">http://reviews.llvm.org/D8351</a><br>
><br>
> Files:<br>
>   CMakeLists.txt<br>
>   <a href="http://Makefile.config.in" target="_blank">Makefile.config.in</a><br>
>   Makefile.rules<br>
>   autoconf/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
>   cmake/modules/<u></u>HandleLLVMOptions.cmake<br>
>   configure<br>
>   docs/CMake.rst<br>
>   include/llvm/ADT/DenseMap.h<br>
>   include/llvm/ADT/EpochTracker.<u></u>h<br>
><br>
> EMAIL PREFERENCES<br>
>   <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
><br>
> ______________________________<u></u>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
><br>
><br>
<br>
</blockquote></div></div>