Horton

Thread

Commits

  1. Avoid macro clash with LLVM 9.

  1. LLVM compile failing in seawasp

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-06T17:32:16Z

    Seawasp (using experimental clang 9.0) has been complaining of late:

/home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -O2  -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include  -I../../../../src/include  -D_GNU_SOURCE -I/usr/include/libxml2  -flto=thin -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
                 from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
                 from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
                 from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
                 from llvmjit_inline.cpp:45:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro "Min" requires 2 arguments, but only 1 given
   : Min(Min), Scalable(Scalable) {}
            ^
In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
                 from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
                 from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
                 from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
                 from llvmjit_inline.cpp:45:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor \xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected \xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
   : Min(Min), Scalable(Scalable) {}
             ^
<builtin>: recipe for target 'llvmjit_inline.o' failed

This was working earlier, and as far as I can tell the cpluspluscheck
fixes are not the cause (because those happened earlier than the first
failure).  Apparently clang got upgraded from "trunk 361691" to "trunk
362290" ... is the new clang broken?

-- 
Álvaro Herrera                            39°50'S 73°21'W

  2. Re: LLVM compile failing in seawasp

    Andres Freund <andres@anarazel.de> — 2019-06-06T17:38:11Z

    Hi,

On 2019-06-06 13:32:16 -0400, Alvaro Herrera wrote:
> Seawasp (using experimental clang 9.0) has been complaining of late:
> 
> /home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -O2  -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include  -I../../../../src/include  -D_GNU_SOURCE -I/usr/include/libxml2  -flto=thin -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
>                  from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro "Min" requires 2 arguments, but only 1 given
>    : Min(Min), Scalable(Scalable) {}
>             ^
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
>                  from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor \xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected \xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
>    : Min(Min), Scalable(Scalable) {}
>              ^
> <builtin>: recipe for target 'llvmjit_inline.o' failed
> 
> This was working earlier, and as far as I can tell the cpluspluscheck
> fixes are not the cause (because those happened earlier than the first
> failure).  Apparently clang got upgraded from "trunk 361691" to "trunk
> 362290" ... is the new clang broken?

I think that machine might also update llvm to a trunk checkout. Is that
right Fabien?  If so that's possible "just" a minor API break.

Greetings,

Andres Freund

  3. Re: LLVM compile failing in seawasp

    didier <did447@gmail.com> — 2019-06-06T17:57:05Z

    c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min member

On Thu, Jun 6, 2019 at 7:32 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> Seawasp (using experimental clang 9.0) has been complaining of late:
>
> /home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -O2  -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include  -I../../../../src/include  -D_GNU_SOURCE -I/usr/include/libxml2  -flto=thin -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
>                  from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro "Min" requires 2 arguments, but only 1 given
>    : Min(Min), Scalable(Scalable) {}
>             ^
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
>                  from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
>                  from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor \xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected \xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
>    : Min(Min), Scalable(Scalable) {}
>              ^
> <builtin>: recipe for target 'llvmjit_inline.o' failed
>
> This was working earlier, and as far as I can tell the cpluspluscheck
> fixes are not the cause (because those happened earlier than the first
> failure).  Apparently clang got upgraded from "trunk 361691" to "trunk
> 362290" ... is the new clang broken?
>
> --
> Álvaro Herrera                            39°50'S 73°21'W
>
>

  4. Re: LLVM compile failing in seawasp

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-06-06T18:35:56Z

    >> failure).  Apparently clang got upgraded from "trunk 361691" to "trunk
>> 362290" ... is the new clang broken?
>
> I think that machine might also update llvm to a trunk checkout. Is that
> right Fabien?

Yes, the version is recompiled from sources on every Saturday.

-- 
Fabien.

  5. Re: LLVM compile failing in seawasp

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-07T00:12:57Z

    didier <did447@gmail.com> writes:
> c.h defines a C Min macro conflicting with llvm new class
> llvm:ElementCount Min member

Really?  Well, we will hardly be the only code they broke with that.
I think we can just wait for them to reconsider.

			regards, tom lane

  6. Re: LLVM compile failing in seawasp

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-27T05:12:19Z

    On Fri, Jun 7, 2019 at 12:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> didier <did447@gmail.com> writes:
> > c.h defines a C Min macro conflicting with llvm new class
> > llvm:ElementCount Min member
>
> Really?  Well, we will hardly be the only code they broke with that.
> I think we can just wait for them to reconsider.

FYI This is now on LLVM's release_90 branch, due out on August 28.

-- 
Thomas Munro
https://enterprisedb.com

  7. Re: LLVM compile failing in seawasp

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-07-27T07:05:58Z

    >>> c.h defines a C Min macro conflicting with llvm new class
>>> llvm:ElementCount Min member
>>
>> Really?  Well, we will hardly be the only code they broke with that.
>> I think we can just wait for them to reconsider.
>
> FYI This is now on LLVM's release_90 branch, due out on August 28.

Maybe we should consider doing an explicit bug report, but I would not bet 
that they are going to fold… or fixing the issue pg side, eg "pg_Min", 
less than 400 hundred instances, and backpatch to all supported 
versions:-(

-- 
Fabien.

  8. Re: LLVM compile failing in seawasp

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-27T07:12:14Z

    On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> >>> c.h defines a C Min macro conflicting with llvm new class
> >>> llvm:ElementCount Min member
> >>
> >> Really?  Well, we will hardly be the only code they broke with that.
> >> I think we can just wait for them to reconsider.
> >
> > FYI This is now on LLVM's release_90 branch, due out on August 28.
>
> Maybe we should consider doing an explicit bug report, but I would not bet
> that they are going to fold… or fixing the issue pg side, eg "pg_Min",
> less than 400 hundred instances, and backpatch to all supported
> versions:-(

I would just #undef Min for our small number of .cpp files that
include LLVM headers.  It's not as though you need it in C++, which
has std::min() from <algorithm>.

-- 
Thomas Munro
https://enterprisedb.com

  9. Re: LLVM compile failing in seawasp

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-27T09:40:35Z

    On Sat, Jul 27, 2019 at 7:12 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > Maybe we should consider doing an explicit bug report, but I would not bet
> > that they are going to fold… or fixing the issue pg side, eg "pg_Min",
> > less than 400 hundred instances, and backpatch to all supported
> > versions:-(
>
> I would just #undef Min for our small number of .cpp files that
> include LLVM headers.  It's not as though you need it in C++, which
> has std::min() from <algorithm>.

Like so.  Fixes the problem for me (llvm-devel-9.0.d20190712).

-- 
Thomas Munro
https://enterprisedb.com

  10. Re: LLVM compile failing in seawasp

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-07-28T07:47:17Z

    Hello Thomas,

>> I would just #undef Min for our small number of .cpp files that
>> include LLVM headers.  It's not as though you need it in C++, which
>> has std::min() from <algorithm>.
>
> Like so.  Fixes the problem for me (llvm-devel-9.0.d20190712).

Hmmm. Not so nice, but if it works, why not, at least the impact is 
much smaller than renaming.

Note that the Min macro is used in several pg headers (ginblock.h, 
ginxlog.h, hash.h, simplehash.h, spgist_private.h), so you might really 
need it depending on what is being done later.

Otherwise, why not simply move llvm C++ includes *before* postgres 
includes? They should be fully independent anyway, so the order should 
not matter?

-- 
Fabien.

  11. Re: LLVM compile failing in seawasp

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-28T13:54:05Z

    Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Otherwise, why not simply move llvm C++ includes *before* postgres 
> includes?

We've been burnt in the past by putting other headers before postgres.h.
(A typical issue is that the interpretation of <stdio.h> varies depending
on _LARGE_FILES or a similar macro, so you get problems if something
causes that to be included before pg_config.h has set that macro.)
Maybe none of the platforms where that's an issue have C++, but that
doesn't seem like a great assumption.

> They should be fully independent anyway, so the order should 
> not matter?

On what grounds do you claim that's true anywhere, let alone
everywhere?

			regards, tom lane

  12. Re: LLVM compile failing in seawasp

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-07-28T20:02:41Z

    Hello Tom,

>> They should be fully independent anyway, so the order should
>> not matter?
>
> On what grounds do you claim that's true anywhere, let alone
> everywhere?

I mean that the intersection of Postgres realm, a database written in C, 
and LLVM realm, a compiler written in C++, should not interfere much one 
with the other, bar the jit compilation stuff which mixes both, so having 
one set of realm-specific includes before/after the other *should* not 
matter.

Obviously the Min macro is a counter example of that, but that is indeed 
the problem to solve, and it is really accidental. It would be very 
unlucky if there was an issue the other way around. But maybe not.

Anyway, I'm just trying to suggest a minimum fuss solution. One point of 
"seawasp" and "jellyfish" is to have early warning of compilation issues 
with future compilers, and it is serving this purpose beautifully. Another 
point is to detect compiler bugs early when compiling a significant 
project, and I have reported issues about both clang & gcc in the past, so 
it works there too.

If reordering includes is not an option, too bad. Then "#undef Min" which 
I find disputable, allthough I've done much worse... it might or might not 
work depending on what is done afterwards. Or rename the macro, as I 
suggested first, but there are many instances. Or convince LLVM people 
that they should change their stuff. Or document that pg jit will cannot 
use the latest LLVM, as a feature. Or find another solution:-)

-- 
Fabien.

  13. Re: LLVM compile failing in seawasp

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-28T21:50:32Z

    On Mon, Jul 29, 2019 at 8:03 AM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> If reordering includes is not an option, too bad. Then "#undef Min" which
> I find disputable, allthough I've done much worse... it might or might not
> work depending on what is done afterwards. Or rename the macro, as I
> suggested first, but there are many instances. Or convince LLVM people
> that they should change their stuff. Or document that pg jit will cannot
> use the latest LLVM, as a feature. Or find another solution:-)

Let's just commit the #undef so that seawasp is green and back to
being ready to tell us if something else breaks.  Personally, I don't
see any reason why <random other project> should entertain a request
to change their variable names to avoid our short common word macros
that aren't even all-caps, but if someone asks them and they agree to
do that before the final 9.0 release we can just revert.

-- 
Thomas Munro
https://enterprisedb.com

  14. Re: LLVM compile failing in seawasp

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-28T21:55:33Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> Let's just commit the #undef so that seawasp is green and back to
> being ready to tell us if something else breaks.

+1.  I was afraid that working around this would be impossibly
painful ... but if it just takes one judiciously placed #undef,
let's do that and not argue about it.

			regards, tom lane

  15. Re: LLVM compile failing in seawasp

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-28T22:27:54Z

    On Mon, Jul 29, 2019 at 9:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > Let's just commit the #undef so that seawasp is green and back to
> > being ready to tell us if something else breaks.
>
> +1.  I was afraid that working around this would be impossibly
> painful ... but if it just takes one judiciously placed #undef,
> let's do that and not argue about it.

Done.

-- 
Thomas Munro
https://enterprisedb.com

  16. Re: LLVM compile failing in seawasp

    Andres Freund <andres@anarazel.de> — 2019-07-29T05:06:41Z

    Hi,

On 2019-07-29 10:27:54 +1200, Thomas Munro wrote:
> On Mon, Jul 29, 2019 at 9:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Thomas Munro <thomas.munro@gmail.com> writes:
> > > Let's just commit the #undef so that seawasp is green and back to
> > > being ready to tell us if something else breaks.
> >
> > +1.  I was afraid that working around this would be impossibly
> > painful ... but if it just takes one judiciously placed #undef,
> > let's do that and not argue about it.
> 
> Done.

cool, thanks.

Greetings,

Andres Freund

  17. Re: LLVM compile failing in seawasp

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-07-29T05:08:21Z

    >>> Let's just commit the #undef so that seawasp is green and back to
>>> being ready to tell us if something else breaks.
>>
>> +1.  I was afraid that working around this would be impossibly
>> painful ... but if it just takes one judiciously placed #undef,
>> let's do that and not argue about it.
>
> Done.

Seawasp is back to green.

-- 
Fabien.