[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I agree with @tianshilei1992, I think we should avoid introducing new `CMAKE_` variables to avoid confusion. The same applies to module names, for example I don't think we should be introducing `GNUBinaryDirs` which can be easily confused for `GNUInstallDirs`.

I would personally go with the `LLVM_` prefix, I agree that this can lead to some confusion given that LLVM is both the name of the top-level project as well as one of the subprojects, but I think that ship has already sailed we already have proliferation of `LLVM_` named variables throughout the codebase.

Regarding `GNUBinaryDirs`, it seems like we always use the following pattern:

  # Must go before the first `include(GNUInstallDirs)`.
  include(LLVMLibdirSuffix)
  
  # Must go below project(..)
  include(GNUInstallDirs)
  include(GNUBinaryDirs)

That introduces a lot of duplication and is potentially error-prone since we rely on particular include order. Could we instead introduce a single `LLVMInstallDirs` module which would contain the content of  `LLVMLibdirSuffix` and `GNUBinaryDirs` and would also include `GNUInstallDirs`, so `LLVMInstallDirs` is all you would need to include in individual subprojects?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608