pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedHeader checking failures on LLVM-less machines
6+ messages / 2 participants
[nested] [flat]
* Header checking failures on LLVM-less machines
@ 2019-01-28 16:19 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2019-01-28 16:19 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Andres Freund <andres@anarazel.de>
Since the LLVM stuff went in, src/tools/pginclude/cpluspluscheck
fails on my main devel machine, because
In file included from /tmp/cpluspluscheck.jobsM6/test.cpp:3:
./src/include/jit/llvmjit_emit.h:13:25: error: llvm-c/Core.h: No such file or directory
and then there's a bunch of ensuing spewage due to missing typedefs
etc. llvmjit.h has the same problem plus this additional pointless
aggravation:
In file included from /tmp/cpluspluscheck.jobsM6/test.cpp:3:
./src/include/jit/llvmjit.h:15:2: error: #error "llvmjit.h should only be included by code dealing with llvm"
I propose that we should fix this by making the whole bodies of those
two headers be #ifdef USE_LLVM.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Header checking failures on LLVM-less machines
@ 2019-01-29 00:51 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2019-01-29 00:51 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
I wrote:
> Since the LLVM stuff went in, src/tools/pginclude/cpluspluscheck
> fails on my main devel machine, because
Actually, it also fails on another machine that does have LLVM installed:
In file included from /tmp/cpluspluscheck.LqnoZj/test.cpp:3:
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_ptr_const(void*, LLVMTypeRef)':
./src/include/jit/llvmjit_emit.h:22:32: error: 'TypeSizeT' was not declared in this scope
LLVMValueRef c = LLVMConstInt(TypeSizeT, (uintptr_t) ptr, false);
^~~~~~~~~
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_sizet_const(size_t)':
./src/include/jit/llvmjit_emit.h:78:22: error: 'TypeSizeT' was not declared in this scope
return LLVMConstInt(TypeSizeT, i, false);
^~~~~~~~~
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_sbool_const(bool)':
./src/include/jit/llvmjit_emit.h:87:22: error: 'TypeStorageBool' was not declared in this scope
return LLVMConstInt(TypeStorageBool, (int) i, false);
^~~~~~~~~~~~~~~
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_pbool_const(bool)':
./src/include/jit/llvmjit_emit.h:96:22: error: 'TypeParamBool' was not declared in this scope
return LLVMConstInt(TypeParamBool, (int) i, false);
^~~~~~~~~~~~~
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_mcxt_switch(LLVMModuleRef, LLVMBuilderRef, LLVMValueRef)':
./src/include/jit/llvmjit_emit.h:205:34: error: 'StructMemoryContextData' was not declared in this scope
cur = LLVMAddGlobal(mod, l_ptr(StructMemoryContextData), cmc);
^~~~~~~~~~~~~~~~~~~~~~~
./src/include/jit/llvmjit_emit.h:205:34: note: suggested alternative: 'MemoryContextData'
cur = LLVMAddGlobal(mod, l_ptr(StructMemoryContextData), cmc);
^~~~~~~~~~~~~~~~~~~~~~~
MemoryContextData
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_funcnullp(LLVMBuilderRef, LLVMValueRef, size_t)':
./src/include/jit/llvmjit_emit.h:223:9: error: 'FIELDNO_FUNCTIONCALLINFODATA_ARGS' was not declared in this scope
FIELDNO_FUNCTIONCALLINFODATA_ARGS,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_funcvaluep(LLVMBuilderRef, LLVMValueRef, size_t)':
./src/include/jit/llvmjit_emit.h:241:9: error: 'FIELDNO_FUNCTIONCALLINFODATA_ARGS' was not declared in this scope
FIELDNO_FUNCTIONCALLINFODATA_ARGS,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Evidently, llvmjit_emit.h doesn't meet the project standard that says
it should be includable standalone (to ensure that header inclusion
order isn't important in .c files). It looks like it needs to #include
llvmjit.h and fmgr.h to satisfy these references. Is there a good
reason why this wasn't done?
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Header checking failures on LLVM-less machines
@ 2019-01-29 00:55 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Andres Freund @ 2019-01-29 00:55 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org
On 2019-01-28 19:51:22 -0500, Tom Lane wrote:
> On 2019-01-28 11:19:21 -0500, Tom Lane wrote:
> > Since the LLVM stuff went in, src/tools/pginclude/cpluspluscheck
> > fails on my main devel machine, because
> >
> > In file included from /tmp/cpluspluscheck.jobsM6/test.cpp:3:
> > ./src/include/jit/llvmjit_emit.h:13:25: error: llvm-c/Core.h: No such file or directory
> >
> > and then there's a bunch of ensuing spewage due to missing typedefs
> > etc. llvmjit.h has the same problem plus this additional pointless
> > aggravation:
> >
> > In file included from /tmp/cpluspluscheck.jobsM6/test.cpp:3:
> > ./src/include/jit/llvmjit.h:15:2: error: #error "llvmjit.h should only be included by code dealing with llvm"
> >
> > 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.
> Actually, it also fails on another machine that does have LLVM installed:
>
> In file included from /tmp/cpluspluscheck.LqnoZj/test.cpp:3:
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_ptr_const(void*, LLVMTypeRef)':
> ./src/include/jit/llvmjit_emit.h:22:32: error: 'TypeSizeT' was not declared in this scope
> LLVMValueRef c = LLVMConstInt(TypeSizeT, (uintptr_t) ptr, false);
> ^~~~~~~~~
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_sizet_const(size_t)':
> ./src/include/jit/llvmjit_emit.h:78:22: error: 'TypeSizeT' was not declared in this scope
> return LLVMConstInt(TypeSizeT, i, false);
> ^~~~~~~~~
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_sbool_const(bool)':
> ./src/include/jit/llvmjit_emit.h:87:22: error: 'TypeStorageBool' was not declared in this scope
> return LLVMConstInt(TypeStorageBool, (int) i, false);
> ^~~~~~~~~~~~~~~
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_pbool_const(bool)':
> ./src/include/jit/llvmjit_emit.h:96:22: error: 'TypeParamBool' was not declared in this scope
> return LLVMConstInt(TypeParamBool, (int) i, false);
> ^~~~~~~~~~~~~
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_mcxt_switch(LLVMModuleRef, LLVMBuilderRef, LLVMValueRef)':
> ./src/include/jit/llvmjit_emit.h:205:34: error: 'StructMemoryContextData' was not declared in this scope
> cur = LLVMAddGlobal(mod, l_ptr(StructMemoryContextData), cmc);
> ^~~~~~~~~~~~~~~~~~~~~~~
> ./src/include/jit/llvmjit_emit.h:205:34: note: suggested alternative: 'MemoryContextData'
> cur = LLVMAddGlobal(mod, l_ptr(StructMemoryContextData), cmc);
> ^~~~~~~~~~~~~~~~~~~~~~~
> MemoryContextData
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_funcnullp(LLVMBuilderRef, LLVMValueRef, size_t)':
> ./src/include/jit/llvmjit_emit.h:223:9: error: 'FIELDNO_FUNCTIONCALLINFODATA_ARGS' was not declared in this scope
> FIELDNO_FUNCTIONCALLINFODATA_ARGS,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./src/include/jit/llvmjit_emit.h: In function 'LLVMOpaqueValue* l_funcvaluep(LLVMBuilderRef, LLVMValueRef, size_t)':
> ./src/include/jit/llvmjit_emit.h:241:9: error: 'FIELDNO_FUNCTIONCALLINFODATA_ARGS' was not declared in this scope
> FIELDNO_FUNCTIONCALLINFODATA_ARGS,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Evidently, llvmjit_emit.h doesn't meet the project standard that says
> it should be includable standalone (to ensure that header inclusion
> order isn't important in .c files). It looks like it needs to #include
> llvmjit.h and fmgr.h to satisfy these references. Is there a good
> reason why this wasn't done?
Not really a good one - it's not really meant as an API just a
collection of mini helpers for codegen, but there's no reason not to
make it self sufficient.
Will make them so.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Header checking failures on LLVM-less machines
@ 2019-01-29 01:21 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2019-01-29 01:21 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
Andres Freund <andres@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.)
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Header checking failures on LLVM-less machines
@ 2019-01-29 01:37 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Andres Freund @ 2019-01-29 01:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org
Hi,
On 2019-01-28 20:21:49 -0500, Tom Lane wrote:
> Andres Freund <andres@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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Header checking failures on LLVM-less machines
@ 2019-01-29 03:19 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lane @ 2019-01-29 03:19 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
Andres Freund <andres@anarazel.de> writes:
> 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 ...?
Works for me now. Thanks for fixing.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2019-01-29 03:19 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 16:19 Header checking failures on LLVM-less machines Tom Lane <tgl@sss.pgh.pa.us>
2019-01-29 00:51 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-01-29 00:55 ` Andres Freund <andres@anarazel.de>
2019-01-29 01:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-01-29 01:37 ` Andres Freund <andres@anarazel.de>
2019-01-29 03:19 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox