[PATCH] D31866: Keep NDEBUG out of header

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: include/llvm/MC/MCSchedule.h:29
   const char *Name;
-#endif
   unsigned NumUnits; // Number of resource of this kind
----------------
jaykang10 wrote:
> mehdi_amini wrote:
> > I don't understand this change.
> > We don't want to increase the runtime memory consumption when not needed. This should still be compiled out in release build.
> > 
> Sorry Mehdi. I know what you mean. In order to remove the "#ifndef NDEBUG" on dumpuses, we also need to remove it here. This patch's goal is to build llvm with cmake option -DCMAKE_CXX_FLAGS:STRING="-DLLVM_ENABLE_DUMP" -DCMAKE_BUILD_TYPE=RelWithDebInfo. First time, I suggested to add "|| defined(LLVM_ENABLE_DUMP)" with "#ifndef NDEBUG" to achieve the goal but I understand what Matthias want. I am not sure how I can satisfy you guys. If possible, can you suggest idea please? I will be very happy with it. 
This is a case where the dumping code seems to be dependent on !NDEBUG code. You could solve this by tweaking the SchedBoundary::dumpScheduledState() implementation instead. (Like for example not calling getResourceName() with NDEBUG enabled).


https://reviews.llvm.org/D31866