Hi,
On 2018-06-10 15:59:04 -0400, Tom Lane wrote:
> I find that the JIT stuff has broken cpluspluscheck for me, along
> with a related script that I use to verify that each header builds
> cleanly standalone (ie with no prerequisites except postgres.h).
> There are two problems:
>
> (1) Doesn't work on a platform without the llvm header files:
>
> ./src/include/jit/llvmjit.h:18:26: error: llvm-c/Types.h: No such file or directory
> followed by a lot of complaints like
> ./src/include/jit/llvmjit.h:60: error: 'LLVMTypeRef' does not name a type
>
> It seems like a reasonable fix for that is to wrap the contents of these
> headers in "#ifdef USE_LLVM" ... do you see a reason not to?
> (2) This seems to need re-thought:
>
> ./src/include/jit/llvmjit.h:15:2: error: #error "llvmjit.h should only be included by code dealing with llvm"
>
> I don't especially see the value of this #error, especially if we are
> wrapping this whole header in "#ifdef USE_LLVM", and propose to just
> remove it.
The reason 1) and 2) happen in this conjunction is that during
development I repeatedly made the mistake of including llvmjit.h, and
then used containing declarations, in files that shouldn't rely on it.
This was to help spot future mistakes of the same kind.
But arguably that kind of has been obsoleted by the more stringent
"plugin" model that's now in place, where llvmjit.h really shouldn't
ever be included outside backend/jit/llvmjit/, at leats in core. Out of
core somebody could reasonably try to layer something ontop of it to
build something more complicated.
So I guess we could just go for an #ifdef USE_LLVM as you suggest. Let
me think about it for a day, and then I'll make it so.
Greetings,
Andres Freund