build-cpp / cmkr

Modern build system based on CMake and TOML.
https://cmkr.build
MIT License
439 stars 27 forks source link

[feature] Add an ‘add-definitions’ option #56

Open gmh5225 opened 2 years ago

gmh5225 commented 2 years ago

Would like to add an add-definitions option. Sometimes compile-definitions doesn't work so well.

mrexodia commented 2 years ago

add_definition is bad practice and I don’t want to generate that. Could you show an example of where compile-definitions doesn’t work, maybe something needs to be fixed there instead.

gmh5225 commented 2 years ago

add_definition is bad practice and I don’t want to generate that. Could you show an example of where compile-definitions doesn’t work, maybe something needs to be fixed there instead.

You can use this project as a test. It mainly refers to the LLVM macro definitions compile-definitions = ["${LLVM_DEFINITIONS}", "${CLANG_DEFINITIONS}"] And then Visual Studio's Intellisense will report an error. ERROR-2022-04-25 165305

But add_definitions(${LLVM_DEFINITIONS}) add_definitions(${CLANG_DEFINITIONS}) is ok.

mrexodia commented 2 years ago

The reason is that you are adding a single flag and not multiple. Using variables isn’t really supported in this way with cmkr, but I’ll give it a try later.

Instead I recommend using: https://github.com/LLVMParty/LLVMCMakeTemplate/blob/master/cmake/LLVM.cmake

You can use include-after and then link to the LLVM-Wrapper target.

gmh5225 commented 2 years ago

Wow. I see. This project looks good. Thank you.

gmh5225 commented 2 years ago

I'm not sure if this can be merged. But It works for me now fix Finally many thanks for this project :)