Обсуждение: quiet inline configure check misses a step for clang
Hi, The current quiet inline test doesn't work for clang. As e.g. evidenced in http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gull&dt=2014-04-03%2007%3A49%3A26&stg=configure configure thinks it's not quiet. Which means that postgres compiled with a recent clang will be noticably slower than it needs to be. The reason for that is that clang is smart and warns about static inline if they are declared locally in the .c file, but not if they are declared in a #included file. That seems to be a reasonable behaviour... I think that needs to be fixed. We either can make the configure test considerably more complex or simply drop the requirement for quiet inline. Comments? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > The current quiet inline test doesn't work for clang. As e.g. evidenced in > http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gull&dt=2014-04-03%2007%3A49%3A26&stg=configure > configure thinks it's not quiet. > Which means that postgres compiled with a recent clang will be noticably > slower than it needs to be. > The reason for that is that clang is smart and warns about static inline > if they are declared locally in the .c file, but not if they are > declared in a #included file. That seems to be a reasonable > behaviour... > I think that needs to be fixed. We either can make the configure test > considerably more complex or simply drop the requirement for quiet > inline. I object to the latter; you're proposing to greatly increase the warning noise seen with any compiler that issues a warning for this without caring about .h vs .c. For somebody who finds gcc -pedantic unusable, I would think you'd have a bit more sympathy for people using other compilers. regards, tom lane
On 2014-04-03 09:43:20 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > The current quiet inline test doesn't work for clang. As e.g. evidenced in > > http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gull&dt=2014-04-03%2007%3A49%3A26&stg=configure > > configure thinks it's not quiet. > > > Which means that postgres compiled with a recent clang will be noticably > > slower than it needs to be. > > > The reason for that is that clang is smart and warns about static inline > > if they are declared locally in the .c file, but not if they are > > declared in a #included file. That seems to be a reasonable > > behaviour... > > > I think that needs to be fixed. We either can make the configure test > > considerably more complex or simply drop the requirement for quiet > > inline. > > I object to the latter; you're proposing to greatly increase the warning > noise seen with any compiler that issues a warning for this without caring > about .h vs .c. For somebody who finds gcc -pedantic unusable, I would > think you'd have a bit more sympathy for people using other compilers. Yea, but which compilers are that? The only one in the buildfarm I could find a couple weeks back was acc, and there's a flag we could add to the relevant template that silences it. I also don't think that very old platforms won't usually be used for active development, so a louder build there doesn't really have the same impact as noisy builds for actively developed on platforms. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2014-04-03 09:43:20 -0400, Tom Lane wrote: >> I object to the latter; you're proposing to greatly increase the warning >> noise seen with any compiler that issues a warning for this without caring >> about .h vs .c. For somebody who finds gcc -pedantic unusable, I would >> think you'd have a bit more sympathy for people using other compilers. > Yea, but which compilers are that? The only one in the buildfarm I could > find a couple weeks back was acc, and there's a flag we could add to the > relevant template that silences it. I also don't think that very old > platforms won't usually be used for active development, so a louder > build there doesn't really have the same impact as noisy builds for > actively developed on platforms. Didn't we already have this discussion last year? The main points are all mentioned in http://www.postgresql.org/message-id/CA+TgmoYJnc4B+8y01gRnAL52GTPbzc3ZsC4SdNW4LGxHqT3Bgg@mail.gmail.com regards, tom lane
On 2014-04-03 09:43:20 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > The current quiet inline test doesn't work for clang. As e.g. evidenced in > > http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gull&dt=2014-04-03%2007%3A49%3A26&stg=configure > > configure thinks it's not quiet. > > > Which means that postgres compiled with a recent clang will be noticably > > slower than it needs to be. > > > The reason for that is that clang is smart and warns about static inline > > if they are declared locally in the .c file, but not if they are > > declared in a #included file. That seems to be a reasonable > > behaviour... > > > I think that needs to be fixed. We either can make the configure test > > considerably more complex or simply drop the requirement for quiet > > inline. > > I object to the latter; Do you have an idea how to make the former work? The whole autoconf getup doesn't really seem to support generating two files for a test. I've looked a bit around and it seems like it'd be a fair amount of effort to do it solely in autoconf. The easiest seems to be to just have a header for the test in the sourcetree, but that seems fairly ugly... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2014-04-03 10:15:33 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-04-03 09:43:20 -0400, Tom Lane wrote: > >> I object to the latter; you're proposing to greatly increase the warning > >> noise seen with any compiler that issues a warning for this without caring > >> about .h vs .c. For somebody who finds gcc -pedantic unusable, I would > >> think you'd have a bit more sympathy for people using other compilers. > > > Yea, but which compilers are that? The only one in the buildfarm I could > > find a couple weeks back was acc, and there's a flag we could add to the > > relevant template that silences it. I also don't think that very old > > platforms won't usually be used for active development, so a louder > > build there doesn't really have the same impact as noisy builds for > > actively developed on platforms. > > Didn't we already have this discussion last year? The main points > are all mentioned in > > http://www.postgresql.org/message-id/CA+TgmoYJnc4B+8y01gRnAL52GTPbzc3ZsC4SdNW4LGxHqT3Bgg@mail.gmail.com To which I replied: http://www.postgresql.org/message-id/20131224141631.GF26564@alap2.anarazel.de It really seems like an odd behaviour if a compiler behaved that way. But even if some decade+ old compiler gets this wrong: I am not going to shed many tears. We're talking about HP-UX's ac++ here. If binaries get a bit more bloated there... I really am not trying to win the inline fight here through the backdoor, I only want to make clang use inlines again. As just written in the other message, I just don't see any easy and nice way to write the autoconf test more robustly. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Hi, On 2014-04-03 12:47:00 +0200, Andres Freund wrote: > The current quiet inline test doesn't work for clang. As e.g. evidenced in > http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gull&dt=2014-04-03%2007%3A49%3A26&stg=configure > configure thinks it's not quiet. > > Which means that postgres compiled with a recent clang will be noticably > slower than it needs to be. > > The reason for that is that clang is smart and warns about static inline > if they are declared locally in the .c file, but not if they are > declared in a #included file. That seems to be a reasonable > behaviour... > > I think that needs to be fixed. We either can make the configure test > considerably more complex or simply drop the requirement for quiet > inline. I still think we really need to fix this. I have three possible solutions: a) Add an external file (in the source tree) that's included in the configure test. b) Have a compiler specific override and specify USE_INLINE there. c) Drop the requirement of quiet inlines. a) would probably the best, but I haven't yet found a non ugly solution :( Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> I still think we really need to fix this. I have three possible
> solutions:
> a) Add an external file (in the source tree) that's included in the
> configure test.
> b) Have a compiler specific override and specify USE_INLINE there.
> c) Drop the requirement of quiet inlines.
> a) would probably the best, but I haven't yet found a non ugly solution :(
Why is (a) so hard again?
(If you're wondering where to put the file, I'd vote for the config/
directory.)
regards, tom lane
On 2014-05-01 10:10:46 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > I still think we really need to fix this. I have three possible > > solutions: > > > a) Add an external file (in the source tree) that's included in the > > configure test. > > b) Have a compiler specific override and specify USE_INLINE there. > > c) Drop the requirement of quiet inlines. > > > a) would probably the best, but I haven't yet found a non ugly solution :( > > Why is (a) so hard again? > > (If you're wondering where to put the file, I'd vote for the config/ > directory.) If we accept that the test includes a external file that's included in the source tree, it's not hard. What'd be hard is to generate a second file via autoconf. I looked at doing the latter and it seemed far to complex. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2014-05-01 10:10:46 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > I still think we really need to fix this. I have three possible > > solutions: > > > a) Add an external file (in the source tree) that's included in the > > configure test. > > b) Have a compiler specific override and specify USE_INLINE there. > > c) Drop the requirement of quiet inlines. > > > a) would probably the best, but I haven't yet found a non ugly solution :( > > Why is (a) so hard again? > > (If you're wondering where to put the file, I'd vote for the config/ > directory.) Patch attached. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
Andres Freund <andres@2ndquadrant.com> writes:
> Patch attached.
Committed with minor comment-smithing.
regards, tom lane
On 2014-05-01 16:17:17 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > Patch attached. > > Committed with minor comment-smithing. Thanks. There's another problematic case: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=olinguito&dt=2014-05-01%2021%3A22%3A43&stg=config configure:9737: ccache gcc -o conftest -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -I/usr/local/include/libxml2-I/usr/local/include -I/usr/include/kerberosV -L/usr/local/lib -lpthread -lcrypto -liconv -L/usr/local/lib-L/usr/local/lib conftest.c -lxslt -lxml2 -lssl -lcrypto -lz -lreadline -ltermcap -lm >&5 /usr/local/lib/libxml2.so.14.0: warning: strcpy() is almost always misused, please use strlcpy() /usr/local/lib/libxml2.so.14.0: warning: strcat() is almost always misused, please use strlcat() /usr/local/lib/libxslt.so.3.8: warning: sprintf() is often misused, please use snprintf() configure:9737: $? = 0 configure: failed program was: That's not new. And it seems like a case of really misguided platform specific changes (actually smelling a bit of zealotry). So I'd be prepared to ignore it. Other opinions? How on earth could it be a good idea to warn about this when linking to libraries instead of warning when a function is actually used during compilation? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > There's another problematic case: > http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=olinguito&dt=2014-05-01%2021%3A22%3A43&stg=config > /usr/local/lib/libxml2.so.14.0: warning: strcpy() is almost always misused, please use strlcpy() > /usr/local/lib/libxml2.so.14.0: warning: strcat() is almost always misused, please use strlcat() > /usr/local/lib/libxslt.so.3.8: warning: sprintf() is often misused, please use snprintf() > That's not new. And it seems like a case of really misguided platform > specific changes (actually smelling a bit of zealotry). So I'd be > prepared to ignore it. Other opinions? Yeah, I've noticed those before, and concluded that "misguided zealotry" is the appropriate classification. I certainly won't do anything about those. (These particular ones, we *can't* do anything about, since the calls are inside libraries we don't control the source of...) > How on earth could it be a good idea to warn about this when linking to > libraries instead of warning when a function is actually used during > compilation? Not only misguided, but crappily implemented zealotry. regards, tom lane
On 2014-05-01 16:17:17 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > Patch attached. > > Committed with minor comment-smithing. Interestingly this seems to have allowed the quiet inline test to succeedd on HP-UX ac++ as well: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=anole&dt=2014-05-02%2023%3A36%3A53&stg=config Seems it uses a similar logic to clang. For a lot longer. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services