Hi Chris,

Thanks for doing this. I've had a quick scan and I have a few minor comments.

# Scripting overview

This is personal preference but I don't like ``add_definitions()`` due
to its global behaviour so I think its use should be discouraged. I
much prefer ``target_compile_definitions()`` which isn't global.

# Dereferencing

One "Gotcha" I think that is worth mentioning is implicit derefencing
of variables in ``if()`` conditionals

for example

```
if ("${SOME_VAR}" STREQUAL "MSVC")
```

behaves very strangely because CMake will implicitly dereference
"MSVC" (as if it was "${MSVC}") where as someone reading the code
probably thinks that it is trying to check if the contents of the
SOME_VAR as a string are "MSVC".

This behaviour is prevented by setting CMP0054 to NEW but this was
only introduced with CMake 3.1 and I don't think that's LLVM's minimum
version so developers might hit this issue. Run ``cmake --help-policy
CMP0054`` for more details.

A hacky work around I employ is to write conditionals like that like this

```
if ("X${SOME_VAR}" STREQUAL "XMSVC")
```

It's not good though...

# Scope

You don't mention "GLOBAL" properties. IIRC LLVM's CMake code uses
these so it might be worth mentioning these

# LLVM specific macros/functions

LLVM's CMake code pretty much avoids using many of the standard CMake
commands for declaring targets favouring its own (sometimes
confusingly named, e.g. add_llvm_library Vs. llvm_add_library) which I
sometime find quite confusing. Seeing as this guide is aimed at LLVM
developers I think this document (or an accompanying document) should
describe these macros/functions.

Thanks,
Dan.
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to