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