[PATCH] D52199: [profile] Install headers for custom runtime maintainers

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 5 02:13:40 PDT 2018


delcypher requested changes to this revision.
delcypher added a comment.
This revision now requires changes to proceed.

@vsk I'm not entirely convinced this is correct but this might be because I don't understand how these header files are meant to be consumed.

Your implementation of `add_compiler_rt_header()` calls `add_compiler_rt_resource_file()` which installs into the Clang resource directory (`<install_root>/share/`). This is not the same path that other sanitizer interface headers get installed (see `include/CMakeLists.txt` in the compiler-rt source tree) which is `<install_root>/include`.

This means that clients of won't be able consume these header files in the same way that they consume other sanitizer header files. Perhaps this is intentional because these header files might be internal headers (I don't know. I've not really looked at the profile portion of compiler-rt before).
If that is your intention I really think you need to rename `add_compiler_rt_header` to something that expresses the intent more explicitly. The name `add_compiler_rt_header` could easily lead someone to believe it will get installed to `<install_root>/include` which doesn't appear to be the case.


https://reviews.llvm.org/D52199





More information about the llvm-commits mailing list