smeenai added inline comments.
================
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()
----------------
phosek wrote:
> 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.
I'm playing a little trick with the lack of underscore. In case you're using
the default unnamed distribution (as in you just specified
`LLVM_DISTRIBUTION_COMPONENTS`), `${distribution}` will be empty, so it'll
expand to just `CLANG_HAS_EXPORTS`, which matches the existing property. If you
wanted to add the underscore, you'd need to special-case the unnamed
distribution, which seemed like a bunch of effort for a property name that
should never be accessed directly. With that being said though, since all the
Add*.cmake usages of `get_llvm_distribution` follow a similar pattern, I was
considering just creating a higher-level function. Lemme make that change and
share what it would look like and then we can decide which way we wanna go.
================
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@
----------------
phosek wrote:
> 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?
I used the lowercase for consistency with the existing
`@llvm_config_include_buildtree_only_exports@` in the LLVM files, but I'm happy
to change it.
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
