n Nov 11, 2013, at 12:09 PM, Alp Toker <email@example.com
>> Even when you have a !NDEBUG build, the platform assert() is pretty
>> crummy on Windows and generates, at best a UTF-16 dump, or
>> just pops up a dialog. WebKit and other projects take the same
>> 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(),
>> to use our own version in order for the definition to match.
> 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
> 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
> 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
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.