[llvm] r231214 - Revert "[ADT] fail-fast iterators for DenseMap"

Zachary Turner zturner at google.com
Wed Mar 4 13:22:16 PST 2015


The _DEBUG idea doesn't work anyway because it's not defined on all
platforms.  I think we should not be having NDEBUG in headers though.
NDEBUG should mean "it's ok to use asserts" not "it's ok to change a
struct's layout".

On Wed, Mar 4, 2015 at 1:17 PM rfoos <rfoos at codeaurora.org> wrote:

> Should _DEBUG be undefined in a release build as well?
>
>
> Sent from my Verizon Wireless 4G LTE smartphone
>
>
> -------- Original message --------
> From: Zachary Turner
> Date:03/04/2015 2:02 PM (GMT-06:00)
> To: Reid Kleckner
> Cc: Commit Messages and Patches for LLVM
> Subject: Re: [llvm] r231214 - Revert "[ADT] fail-fast iterators for
> DenseMap"
>
> I can confirm that removing the ABI incompatibility fixes the issue for me
> locally.  I tested by commenting out the NDEBUG path in DebugEpochBase.h so
> that it always used the debug path, and then rebuilding everything.
>
> Can we change this to be _DEBUG instead of NDEBUG?
>
> On Wed, Mar 4, 2015 at 11:42 AM Reid Kleckner <rnk at google.com> wrote:
>
> > On Wed, Mar 4, 2015 at 9:43 AM, Zachary Turner <zturner at google.com>
> wrote:
> >
> >> How necessary to this change is the ABI incompatibility between the
> cases
> >> where NDEBUG is and isn't defined?  Is it possible to make them ABI
> >> compatible while still keeping the meat of the change intact?
> >>
> >> It doesn't really seem like a "nice" thing to do to require that people
> >> who use LLVM as a library use the same compilation flags as LLVM.
> >>
> >
> > Early in the history of LLVM, we allowed NDEBUG to control struct layout
> > and general ABI. Then Alp Toker came along and decided this was a Bad
> > Thing. He audited the headers and removed it all. Now it's crept back in.
> >
> > In conclusion, what isn't tested will break.
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150304/9f7bf15a/attachment.html>


More information about the llvm-commits mailing list