[PATCH] D11833: s/NDEBUG/LLVM_NDEBUG/ in most places

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

Stepping back to look at the goals:

> The goal of this change is to remove the dependency on NDEBUG in
>  headers, so that library users can freely choose whether or not to
>  define this value when building their applications.

This seems like the same goal that introducing `LLVM_ENABLE_ABI_BREAKING_CHECKS` had.  Is this patch a change in direction for how to achieve the goal?  Or is there a subtlety here, and it's achieving something additional that `LLVM_ENABLE_ABI_BREAKING_CHECKS` does not?  I think it at least deserves an llvm-dev discussion before proceeding (I just took a look through the archives and don't see one, apologies if I missed it).

Note that there are some nice benefits to the existing `LLVM_ENABLE_ABI_BREAKING_CHECKS` model:

- A developer that needs a stable ABI can configure `LLVM_ENABLE_ABI_BREAKING_CHECKS` (either on or off) to achieve that.
- If not specified explicitly, `LLVM_ENABLE_ABI_BREAKING_CHECKS` is controlled by whether it's an "asserts" build.  This gives the right behaviour for most developers most of the time.
- The common case of assertions that are //not// ABI-breaking don't need special treatment.
  - Always controlled by whether it's an "asserts" build.
  - Uses the syntax that people expect.

I think the llvm-dev discussion would need to establish (a) why this isn't working (but the new thing would work) and/or (b) what additional benefits splitting off `LLVM_NDEBUG` and `llvm_assert` would solve.  It's also not clear what's supposed to happen for other LLVM sub-projects, like Clang.  Should they grow `clang_assert` or use `llvm_assert`?  (BTW, it would probably be nice to also introduce an `llvm_abi_breaking_assert` or something to simplify the existing model, whether or not this change moves forward.)

I also think changes like this should be documented in the coding standards; I'm requesting changes on that basis.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D11833/new/

https://reviews.llvm.org/D11833