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

[Alp Toker]

On 03/01/2014 22:22, Chandler Carruth wrote:
>
> On Fri, Jan 3, 2014 at 5:08 PM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>
>         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.
>
>
> If we're going to have structural differences between an asserts and a 
> no-asserts build, why should we stress about minimizing them? That is, 
> why not just s/NDEBUG/LLVM_NDEBUG/g (conceptually, not textually of 
> course) and provide a per-build stable idea of whether "asserts" are 
> on or off?
>
We should focus on minimizing them because clang is already 100% stable, 
and LLVM can be there too with a similar effort.

Until LLVM gets there, the 3.5 llvm_config.h approach provides 
sufficient stability. In practice, clang is now already ABI stable 
because it doesn't use the variably-sized structures in its most useful 
interfaces (though I haven't announced this yet as we need some way to 
verify it).

Most of the NDEBUG-modified structures are deep in LLVM CodeGen headers 
so it's inconsequential if they ever become stabilised as far as I'm 
concerned. These are no different to source files and can as well be 
moved to lib/.

If you look at the clang changes achieving all of this, they ended up 
being a great cleanup and I've got a feeling the effort enhanced error 
recovery.

We've had a great push for enhanced diagnostics in the last couple of 
months, and just throwing in an assert or adding a NDEBUG struct field 
is no longer accepted in polite clang circles. This goal is part of that 
story :-)

> That would seem way simpler, and would allow library users to define 
> NDEBUG which ever way they wanted without any impact on LLVM. It would 
> also allow us to not think about whether or not the uses of NDEBUG 
> that are left in the headers are "safe" ODR violations (something I'm 
> pretty suspect about to begin with in an LTO-enabled world).

I'm not OK with adding new NDEBUGs to any headers. Any NDEBUG is 
guaranteed to yield the wrong result 50% of the time when included in an 
external project, so it's never valid to add a new one. If you need 
this, there's almost certainly a better way to achieve it and you can 
ask on the list for suggestions.

API / ABI stability aside, NDEBUG definitions headers have done little 
good and a lot of harm in getting LLVM usable as a library. We're 
missing out as long as we make embedding difficult and this has broken 
for most of LLVM's lifetime -- it's only now that we're now starting to 
get results and some degree of stability on ToT and this is something to 
be excited about.

Alp.


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