phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.
LGTM with a few nits.
================
Comment at: clang/cmake/modules/AddClang.cmake:121
+ set(export_to_clangtargets EXPORT Clang${distribution}Targets)
+ set_property(GLOBAL PROPERTY CLANG${distribution}_HAS_EXPORTS True)
endif()
----------------
Should we make this uppercase and join it with `_` to match the existing
conventions? For example, you'd have `ClangToolchainTargets` and
`CLANG_TOOLCHAIN_HAS_EXPORTS`. With the change as is, we would end up with
`CLANGToolchain_HAS_EXPORT` which is a bit unusual.
================
Comment at: lld/cmake/modules/LLDConfig.cmake.in:13
# Provide all our library targets to users.
-include("@LLD_CONFIG_EXPORTS_FILE@")
+@lld_config_include_exports@
----------------
We use upper-case variables for the `*_CONFIG_CODE` so using lower case here is
a bit unfortunate, could we use upper case for these variables?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89177/new/
https://reviews.llvm.org/D89177
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
