[PATCH] D120714: [CodeGen] Place SDNode debug ID declaration under ifndef

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

dexonsmith added a comment.

In D120714#3375195 <https://reviews.llvm.org/D120714#3375195>, @kovdan01 wrote:

> In D120714#3353070 <https://reviews.llvm.org/D120714#3353070>, @craig.topper wrote:
>
>> Here is a previous thread on this topic https://lists.llvm.org/pipermail/llvm-dev/2016-July/101842.html
>
> Thanks! Inspected that, and found an interesting patchset http://www.nuanti.com/tmp/llvm-api-stability/ in discussion https://lists.llvm.org/pipermail/llvm-dev/2013-November/067463.html. And that was not the only attempt to introduce `LLVM_NDEBUG` - one more was https://reviews.llvm.org/D11833.
>
> In the last review that I mentioned @dexonsmith said that we can use `LLVM_ENABLE_ABI_BREAKING_CHECKS` instead of `NDEBUG` in headers. In context of the current patch that will look unsuitable IMHO - `PersistentId` is only used for debug printing, and a person might want to have `PersistentId` defined in debug build with ABI breaking checks disabled.
>
> As for me, looks like we can try to introduce `LLVM_NDEBUG` one more time :) @craig.topper, @dexonsmith, what do you think about it?

`LLVM_ENABLE_ABI_BREAKING_CHECKS` is the right thing to do here. This is exactly the case it exists for. If people don't want ABI-breaking things in their release builds, they won't (or shouldn't) turn on LLVM_ENABLE_ABI_BREAKING_CHECKS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120714