[llvm] [llvm-c] Guard include of llvm-config in Visibility.h (PR #154229)
Saleem Abdulrasool via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 18 17:49:40 PDT 2025
compnerd wrote:
> > Please extract the `__has_include` check separately. I don't think that this should be guarded by `#if defined(__has_include)`.
>
> Sorry, I'm not sure I understand. Are you suggesting using `has_include` directly without checking if the operator is supported? If so, why?
I'm suggesting that we do something like:
```c
#if !defined(__has_include)
#define __has_include(include) 0
#endif
```
But, we should also guard it with `__has_include(...) || 1`.
> > What about the alternative: we install `llvm-config.h`? I think that makes sense as we don't know what the configuration of the library is (is it shared or static?).
>
> I'd like to avoid pulling in more dependencies if possible. We don't actually need Visibility.h either, since it's not currently necessary or advantageous for us to have these annotated in the preexisting APIs with `LLVM_C_ABI` for our platform. But since it's already bundled with `llvm-c`, it's not too much of a hassle.
>
> This is specifically regarding for how the llvm-c headers get installed and then used by specific clients, not to build LLVM/compilers themselves.
I don't think it is pulling in more dependencies. The header is a single standalone header that gives very specific information that the user *must* provide or the build is invalid (aka, we can invoke UB). You need to indicate if you are building against a static version of the library or not because that impacts code generation on some platforms. If we don't simply add the single CMake generated header, we should provide default values for the provided macros in the configuration.
https://github.com/llvm/llvm-project/pull/154229
More information about the llvm-commits
mailing list