[PATCH] D142363: [cmake] Stop using add_definitions

[Chris Bieneman via Phabricator via llvm-commits]

beanz added inline comments.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:71
-    # `add_definitions` does not support generator expressions.
     add_compile_options($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-UNDEBUG>)
     if (MSVC)
----------------
foad wrote:
> barannikov88 wrote:
> > I wonder if `remove_definitions($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:NDEBUG>)` works?
> > It seems it is able to remove previously added DDEBUG from the command line, but otherwise does not pass "-UNDEBUG" to the compiler.
> > 
> Interesting. I did not know about remove_definitions. But I'd prefer not to do that in this patch, since I have already tested it as-is.
Generator expressions may be a bit convoluted to read, but I think it is better to use `add_compile_options` with a conditional generator expression than to add the option, then need to remove it from places where it shouldn't be.

I think the code as written is preferable and easier to maintain in the long run.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142363