Hi everyone, we have several header files in which the NDEBUG macro is being used. In some of these, the NDEBUG macro changes the size, the interface, or the layout of a type. One example is AssertingVH, which turns into a flat, transparent Value* wrapper in Release build. Now everywhere you use these headers, the state of the NDEBUG flag must be the same, or else bad things could happen. This turns out to be quite problematic for out-of-tree targets and projects, because it effectively forces them to use the same build type with which LLVM was compiled. Thus, a project maintainer needs to keep a Debug and a Release build of LLVM around, and is mostly barred from using binary distributions. If the community agrees that this is a problem, I'd like to discuss two possible solutions. Solution #1: Don't use NDEBUG (or any similar or depending macro) inside public-facing header files. I think this is the cleanest solution, but it requires a great amount of discipline on the side of developers and reviewers to stick with this pattern, and it will probably have a performance impact (e.g., for AssertingVH). Solution #2: Replace NDEBUG with a custom flag (e.g. LLVM_NDEBUG), and have CMake take care of synchronizing the two. I.e. ensure that LLVM_NDEBUG is set iff NDEBUG is set. Introducing this is probably a lot more work, because it touches all subprojects. What do you think? Best, - Philip -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160701/f85af868/attachment.html>
When we added the ABI breaking EpochTracker to DenseMap, we added a macro called LLVM_ENABLE_ABI_BREAKING_CHECKS, which is controlled by the CMake option LLVM_ABI_BREAKING_CHECKS. I think we should make all ABI-impacting uses of NDEBUG in headers use this macro instead. I don't know what the current behavior is, but I would like this macro to correlate with NDEBUG by default. People packaging LLVM as a library for redistribution can turn off the ABI breaking checks manually. Also, this has come up before, although Alp was concerned with Clang, not LLVM: http://lists.llvm.org/pipermail/llvm-dev/2013-November/067410.html On Fri, Jul 1, 2016 at 5:37 AM, Philip Pfaffe via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hi everyone, > > we have several header files in which the NDEBUG macro is being used. In > some of these, the NDEBUG macro changes the size, the interface, or the > layout of a type. One example is AssertingVH, which turns into a flat, > transparent Value* wrapper in Release build. Now everywhere you use these > headers, the state of the NDEBUG flag must be the same, or else bad things > could happen. > > This turns out to be quite problematic for out-of-tree targets and > projects, because it effectively forces them to use the same build type > with which LLVM was compiled. Thus, a project maintainer needs to keep a > Debug and a Release build of LLVM around, and is mostly barred from using > binary distributions. > > If the community agrees that this is a problem, I'd like to discuss two > possible solutions. > > Solution #1: > Don't use NDEBUG (or any similar or depending macro) inside public-facing > header files. I think this is the cleanest solution, but it requires a > great amount of discipline on the side of developers and reviewers to stick > with this pattern, and it will probably have a performance impact (e.g., > for AssertingVH). > > Solution #2: > Replace NDEBUG with a custom flag (e.g. LLVM_NDEBUG), and have CMake take > care of synchronizing the two. I.e. ensure that LLVM_NDEBUG is set iff > NDEBUG is set. Introducing this is probably a lot more work, because it > touches all subprojects. > > What do you think? > > Best, > - Philip > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160701/ec5b2f8c/attachment.html>
This has also come up with llvm before: http://reviews.llvm.org/D11833 <http://reviews.llvm.org/D11833> - Matthias> On Jul 1, 2016, at 10:11 AM, Reid Kleckner via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > When we added the ABI breaking EpochTracker to DenseMap, we added a macro called LLVM_ENABLE_ABI_BREAKING_CHECKS, which is controlled by the CMake option LLVM_ABI_BREAKING_CHECKS. I think we should make all ABI-impacting uses of NDEBUG in headers use this macro instead. > > I don't know what the current behavior is, but I would like this macro to correlate with NDEBUG by default. People packaging LLVM as a library for redistribution can turn off the ABI breaking checks manually. > > Also, this has come up before, although Alp was concerned with Clang, not LLVM: > http://lists.llvm.org/pipermail/llvm-dev/2013-November/067410.html <http://lists.llvm.org/pipermail/llvm-dev/2013-November/067410.html> > > On Fri, Jul 1, 2016 at 5:37 AM, Philip Pfaffe via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > Hi everyone, > > we have several header files in which the NDEBUG macro is being used. In some of these, the NDEBUG macro changes the size, the interface, or the layout of a type. One example is AssertingVH, which turns into a flat, transparent Value* wrapper in Release build. Now everywhere you use these headers, the state of the NDEBUG flag must be the same, or else bad things could happen. > > This turns out to be quite problematic for out-of-tree targets and projects, because it effectively forces them to use the same build type with which LLVM was compiled. Thus, a project maintainer needs to keep a Debug and a Release build of LLVM around, and is mostly barred from using binary distributions. > > If the community agrees that this is a problem, I'd like to discuss two possible solutions. > > Solution #1: > Don't use NDEBUG (or any similar or depending macro) inside public-facing header files. I think this is the cleanest solution, but it requires a great amount of discipline on the side of developers and reviewers to stick with this pattern, and it will probably have a performance impact (e.g., for AssertingVH). > > Solution #2: > Replace NDEBUG with a custom flag (e.g. LLVM_NDEBUG), and have CMake take care of synchronizing the two. I.e. ensure that LLVM_NDEBUG is set iff NDEBUG is set. Introducing this is probably a lot more work, because it touches all subprojects. > > What do you think? > > Best, > - Philip > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160701/7b2995e9/attachment.html>