[PATCH] D19907: cmake: Prefix Polly options with LLVM_ to avoid variable shadowing

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 13:13:07 PDT 2016


grosser added a comment.

In http://reviews.llvm.org/D19907#420937, @Meinersbur wrote:

> This Differential does a bit more than renaming the cmake variables, which I think it should be reflected in the commit message/title.


As discussed in our phone call, I added another sentence to the commit message to make this clear.

> Each of the LLVM projects usually has its own prefix: "CLANG_",  "LLD_", ... . However, this is LLVM's CMakeLists.txt. I therefore suggest to use "LLVM_POLLY_LINK_INTO_TOOLS" and "LLVM_POLLY_BUILD" such that they are grouped together in ccmake/cmake-gui.


I used the names you suggested.

> Instead of "LLVM_WITH_POLLY" I'd use the already existing "LLVM_TOOL_POLLY_BUILD", or maybe there is an intended difference?

> 

> When  LLVM_EXTERNAL_*BUILD was renamed to LLVM_TOOL_*_BUILD, the old _BUILD property was still respected in order to allow to switch between older and newer revisions (eg. for bisect). We could do the same here, but personally I don't think it is necessary.


This clearly is worth improving, but as this is more involved, it is probably better to leave this for another patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D19907





More information about the llvm-commits mailing list