llvm dev - Jan 2014 - [LLVMdev] [cfe-dev] Goal for 3.5: Library-friendly headers

On Nov 11, 2013, at 12:09 PM, Alp Toker <alp at nuanti.com> wrote:
>> Even when you have a !NDEBUG build, the platform assert() is pretty
>> crummy on Windows and generates, at best a UTF-16 dump, or sometimes
>> just pops up a dialog. WebKit and other projects take the same approach
>> and define their own assertion macros to deal with this portably.
>> 
>> So as far as I can tell, as long as the headers use assert(), they need
>> to use our own version in order for the definition to match.
> 
> Chris,
> 
> Having said that, I agree it's worth trying without llvm_assert() to
see
> how far it gets.
> 
> I'll pull together a rework of this patchset with just the build system
> and structure changes alone to see how far it gets.
> 
> The key thing then is to make sure that it's safe to enable the
> assertions in the headers if an application is built with !NDEBUG and
> linked against an NDEBUG version of LLVM.
Sounds great.  I'm pretty confident that there will be no problems - in
practice - from any ODR violations that might arise from "assert"
differing across library boundaries.  We would want some pretty strong practical
justification for breaking away from standard assert.

-Chris

On Tue, Nov 12, 2013 at 1:20 AM, Chris Lattner <clattner at apple.com>
wrote:
> n Nov 11, 2013, at 12:09 PM, Alp Toker <alp at nuanti.com> wrote:
>
> >> Even when you have a !NDEBUG build, the platform assert() is
pretty
> >> crummy on Windows and generates, at best a UTF-16 dump, or
sometimes
> >> just pops up a dialog. WebKit and other projects take the same
approach
> >> and define their own assertion macros to deal with this portably.
> >>
> >> So as far as I can tell, as long as the headers use assert(), they
need
> >> to use our own version in order for the definition to match.
> >
> > Chris,
> >
> > Having said that, I agree it's worth trying without llvm_assert()
to see
> > how far it gets.
> >
> > I'll pull together a rework of this patchset with just the build
system
> > and structure changes alone to see how far it gets.
> >
> > The key thing then is to make sure that it's safe to enable the
> > assertions in the headers if an application is built with !NDEBUG and
> > linked against an NDEBUG version of LLVM.
>
> Sounds great.  I'm pretty confident that there will be no problems - in
> practice - from any ODR violations that might arise from "assert"
differing
> across library boundaries.  We would want some pretty strong practical
> justification for breaking away from standard assert.

Sorry to dig up this thread, but when re-reading it, I was surprised that
everyone seems to think this will be easily done across all of LLVM.

How can we support AssertingVH, which behaves as a POD-like struct around a
pointer in NDEBUG, and as a class with significant (important)
functionality to implement asserts on dangling value handles in !NDEBUG
builds?

While having different components of LLVM and consumers of LLVM able to
intermix NDEBUG and !NDEBUG built code freely without ABI issues is
nice-to-have in my book, the functionality provided by AssertingVH is
significantly more nice-to-have, and I don't see any easy ways to contain
or limit the exposure of this facility to library-level consumers.

We also have quite a few places in LLVM today that conserve memory usage in
non-assert builds by removing unnecessary members of classes. We can say
that this makes the ABI more fragile, but it is a valuable space
optimization. Chris, are you saying to strongly believe that these should
only be allowed for classes that are not visible in the C++ API? I find
that surprising, as LLVM has historically prioritized efficiency and
developer tools over ABI stability in our C++ APIs. Build-level instability
is certainly a different beast from version stability, but I wanted to make
sure the ramifications of this shift were being considered by everyone.
(They hadn't by me.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140103/44c41c65/attachment.html>

On Jan 3, 2014, at 1:55 PM, Chandler Carruth <chandlerc at google.com>
wrote:
> >
> > The key thing then is to make sure that it's safe to enable the
> > assertions in the headers if an application is built with !NDEBUG and
> > linked against an NDEBUG version of LLVM.
> 
> Sounds great.  I'm pretty confident that there will be no problems - in
practice - from any ODR violations that might arise from "assert"
differing across library boundaries.  We would want some pretty strong practical
justification for breaking away from standard assert.
> 
> Sorry to dig up this thread, but when re-reading it, I was surprised that
everyone seems to think this will be easily done across all of LLVM.
> 
> How can we support AssertingVH, which behaves as a POD-like struct around a
pointer in NDEBUG, and as a class with significant (important) functionality to
implement asserts on dangling value handles in !NDEBUG builds?
> 
> While having different components of LLVM and consumers of LLVM able to
intermix NDEBUG and !NDEBUG built code freely without ABI issues is nice-to-have
in my book, the functionality provided by AssertingVH is significantly more
nice-to-have, and I don't see any easy ways to contain or limit the exposure
of this facility to library-level consumers.
I hadn’t considered AssertVH, and I agree that losing it isn’t an option.

Would it be possible to redesign AssertVH to be non-ABI fragile across
debug/release builds?  I haven’t looked at it recently, but maybe it could be a
pointer to a CallbackVH in the debug mode, or a PointerUnion<rawpointer,
callbackvh> or something.
> We also have quite a few places in LLVM today that conserve memory usage in
non-assert builds by removing unnecessary members of classes. We can say that
this makes the ABI more fragile, but it is a valuable space optimization. Chris,
are you saying to strongly believe that these should only be allowed for classes
that are not visible in the C++ API? I find that surprising, as LLVM has
historically prioritized efficiency and developer tools over ABI stability in
our C++ APIs. Build-level instability is certainly a different beast from
version stability, but I wanted to make sure the ramifications of this shift
were being considered by everyone. (They hadn't by me.)
Do you have any specific examples in mind?

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140103/d061eb1c/attachment.html>

On 03/01/2014 21:55, Chandler Carruth wrote:
>
> On Tue, Nov 12, 2013 at 1:20 AM, Chris Lattner <clattner at apple.com 
> <mailto:clattner at apple.com>> wrote:
>
>     n Nov 11, 2013, at 12:09 PM, Alp Toker <alp at nuanti.com
>     <mailto:alp at nuanti.com>> wrote:
>
>     >> Even when you have a !NDEBUG build, the platform assert() is
pretty
>     >> crummy on Windows and generates, at best a UTF-16 dump, or
>     sometimes
>     >> just pops up a dialog. WebKit and other projects take the same
>     approach
>     >> and define their own assertion macros to deal with this
portably.
>     >>
>     >> So as far as I can tell, as long as the headers use assert(),
>     they need
>     >> to use our own version in order for the definition to match.
>     >
>     > Chris,
>     >
>     > Having said that, I agree it's worth trying without
>     llvm_assert() to see
>     > how far it gets.
>     >
>     > I'll pull together a rework of this patchset with just the
build
>     system
>     > and structure changes alone to see how far it gets.
>     >
>     > The key thing then is to make sure that it's safe to enable
the
>     > assertions in the headers if an application is built with
>     !NDEBUG and
>     > linked against an NDEBUG version of LLVM.
>
>     Sounds great.  I'm pretty confident that there will be no problems
>     - in practice - from any ODR violations that might arise from
>     "assert" differing across library boundaries.  We would want
some
>     pretty strong practical justification for breaking away from
>     standard assert.
>
>
> Sorry to dig up this thread, but when re-reading it, I was surprised 
> that everyone seems to think this will be easily done across all of LLVM.
It is almost done. It was far easier than the original patch.
>
> How can we support AssertingVH, which behaves as a POD-like struct 
> around a pointer in NDEBUG, and as a class with significant 
> (important) functionality to implement asserts on dangling value 
> handles in !NDEBUG builds?
This was in the original patch. If you have a moment I recommend that 
you take a look, particularly at the config.h change which is still the 
current 3.5 plan (the llvm_assert changes which make the bulk of the 
patch aren't in the plan for now though).

To summarise, it's a matter of replacing the 5 or so structurally 
significant NDEBUG with a LLVM_DEBUG_BUILD macro definition in the 
generated llvm-config.h header.
>
> While having different components of LLVM and consumers of LLVM able 
> to intermix NDEBUG and !NDEBUG built code freely without ABI issues is 
> nice-to-have in my book, the functionality provided by AssertingVH is 
> significantly more nice-to-have, and I don't see any easy ways to 
> contain or limit the exposure of this facility to library-level consumers.
As explained above, we will retain AssertingVH exactly as is, with the 
structure adapting exactly as it does now to include extra bits and 
asserts between DEBUG and NDEBUG.

The only difference is that the macro definition will be in llvm-debug.h 
so it'll be stable independent of external project configurations.

>
> We also have quite a few places in LLVM today that conserve memory 
> usage in non-assert builds by removing unnecessary members of classes. 
> We can say that this makes the ABI more fragile, but it is a valuable 
> space optimization.
These can also use the above scheme to achieve embedding stability.

In practice though in the clang headers we found that most of the fields 
were harmless, adding no more than one pointer's worth to a process 
singleton class. Which is really not even a drop in the ocean.

> Chris, are you saying to strongly believe that these should only be 
> allowed for classes that are not visible in the C++ API? I find that 
> surprising, as LLVM has historically prioritized efficiency and 
> developer tools over ABI stability in our C++ APIs. Build-level 
> instability is certainly a different beast from version stability, but 
> I wanted to make sure the ramifications of this shift were being 
> considered by everyone. (They hadn't by me.)
I think you misunderstood the approach. Obviously I'm not going to 
propose something that drastically changes the way structures are 
defined right now.

The early patch, despite it's bulk, covers all of this and is a good 
place to check if you want to see the exact implementation approach..

I also recommend that you check the commits that got clang NDEBUG-free 
in headers.

They're both pretty neat patchsets and it's amazing we got the results 
we wanted with such a small delta.

Alp.

-- 
http://www.nuanti.com
the browser experts