Hi, On 2019-01-28 20:21:49 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2019-01-28 19:51:22 -0500, Tom Lane wrote: > >> I propose that we should fix this by making the whole bodies of those > >> two headers be #ifdef USE_LLVM. > > > Hm, I think it's sufficient that we error out if llvm was configured, > > we've sufficient buildfarm machines running with it enabled. > > That is not the point. The point is that you've broken a developer > tool. I don't really care whether cpluspluscheck would work on > some subset of buildfarm machines --- what I need is for it to work > on *my* machine. It is not any more okay for the llvm headers to fail > like this than it would be for libxml- or openssl- or Windows-dependent > headers to fail on machines lacking respective infrastructure. > > (And yes, I realize that that means that cpluspluscheck can only > check a subset of the header declarations on any particular machine. > But there's no way around that.)
I don't think we are actually contradicting each other. The aim of that error was to prevent pieces of code that aren't conditional on --with-llvm from including llvmjit.h. I was, for a moment and wrongly, thinking that we could style the header in a way that'd make it both for safe for cpluspluscheck and still error in that case, but that was a brainfart. But even leaving that brainfart aside, I'm not arguing against adding those include guards, so ...? I think the raison d'etre for that error has shrunk considerably anyway - it was introduced before the LLVM including/linking code was separated into it's own .so / directory. Greetings, Andres Freund
