[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

Junchang Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 17:24:46 PST 2023


paperchalice added a comment.

In D141907#4094748 <https://reviews.llvm.org/D141907#4094748>, @MaskRay wrote:

> The CMake part of this patch improves the matter. Manually constructed resource dir (many duplicates) string is replace with a library function call.
>
> However, the introduced conditions in C++ code like
>
>   std::string path_to_clang_dir = std::string(CLANG_RESOURCE_DIR).empty()
>                                       ? "/foo/bar/" LLDB_INSTALL_LIBDIR_BASENAME
>                                         "/clang/" CLANG_VERSION_MAJOR_STRING
>                                       : "/foo/bar/bin/" CLANG_RESOURCE_DIR;
>
> makes me uncomfortable.
>
> I wish that we can just replace all manually constructed `CLANG_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING` with a simple `CLANG_RESOURCE_DIR` which is guaranteed to be non-empty.
>
> edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable `CLANG_RESOURCE_DIR` but did not explain why. 
> In the long term, the CMake variable `CLANG_RESOURCE_DIR` probably should be removed.

clang::driver::Driver::GetResourcesPath <https://clang.llvm.org/doxygen/classclang_1_1driver_1_1Driver.html#acda8dfdf4f80efa84df98157e1779152> would be a better choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907



More information about the cfe-commits mailing list