Compiling CVS HEAD with clang under OSX
in [PgSql]
|
Prev: [HACKERS] Compiling CVS HEAD with clang under OSX
Next: english parser in text search: support for multiplewords in the same position
From: Tom Lane on 1 Aug 2010 20:25 Neil Conway <neil.conway(a)gmail.com> writes: > I tried $subject recently, and noticed some minor issues: > (1) Two warnings that suggest bugs; in src/backend/utils/adt, > datetime.c:3101:27: warning: use of logical || with constant operand; > switch to bitwise | or remove constant > And similarly for src/interfaces/ecpg/pgtypeslib/interval.c. Attached > is a patch that replaces logical OR with bitwise OR, which seems to be > the intended coding. Yeah, this seems like an ancient typo. Will apply. > (2) clang doesn't support (or require) "-no-cpp-precomp", which > src/template/darwin adds to $CC unconditionally. > ... > (As an aside, is "no-cpp-precomp" still necessary for > reasonably-modern versions of Apple GCC?) Good question, but before thinking about removing it, we'd have to agree what is a "reasonably modern" version of Apple's gcc. OSX 10.4 is still in use in the wild, for sure. Is 10.3 still interesting? > (3) There are countless warnings emitted during the compilation of > regcomp.c and related files, due to unused values returned by ERR(), > VERR(), FAILW(), and similar macros. Perhaps it is possible to rewrite > the macros to avoid the warning, although I didn't see an easy way to > do that. Perhaps a tack like this would shut it up? #define VERR(vv,e) ((vv)->nexttype = EOS, \ (vv)->err = ((vv)->err ? (vv)->err : (e))) What are you doing exactly to build with clang ... is "CC = clang" sufficient? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 1 Aug 2010 20:56 Neil Conway <neil.conway(a)gmail.com> writes: > *** src/backend/utils/adt/datetime.c 9 May 2010 02:15:59 -0000 1.212 > --- src/backend/utils/adt/datetime.c 1 Aug 2010 23:09:30 -0000 > *************** > *** 3098,3104 **** > break; > case RESERV: > ! tmask = (DTK_DATE_M || DTK_TIME_M); > *dtype = val; > break; > --- 3098,3104 ---- > break; > case RESERV: > ! tmask = (DTK_DATE_M | DTK_TIME_M); > *dtype = val; > break; BTW, some digging in the code shows that this line is reached only for interval input "invalid", which hasn't been accepted for a long time anyhow: regression=# select 'invalid'::interval; ERROR: date/time value "invalid" is no longer supported However, the mistaken value given to tmask interferes with detecting other errors, in particular the DTERR_BAD_FORMAT that you ought to get for specifying conflicting fields. The DTK_DATE_M | DTK_TIME_M value is intended to say that "invalid" conflicts with any date or time field. But since the wrong value is assigned, you can do this: regression=# select '3 day invalid'::interval; ERROR: date/time value "3 day invalid" is no longer supported and the syntax error fails to trigger, so that control gets as far as complaining about the DTK_INVALID type instead. With the fix, you get the intended behavior: regression=# select '3 day invalid'::interval; ERROR: invalid input syntax for type interval: "3 day invalid" So this is a real bug, though surely one that's awfully far down the severity scale, to the point that the compiler warning might be judged more annoying than the actual bug. I'm inclined to fix it in HEAD and 9.0, which are the only branches that anyone's likely to be excited about building with clang. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 1 Aug 2010 22:40 Neil Conway <neil.conway(a)gmail.com> writes: > I tried $subject recently, and noticed some minor issues: I tried to duplicate your results using what I believe to be the latest version of clang, $ clang -v Apple clang version 1.5 (tags/Apple/clang-60) Target: x86_64-apple-darwin10 Thread model: posix (this is a 10.6.4 machine with the Xcode update that came out last week). I got some curious discrepancies from your report. > (1) Two warnings that suggest bugs; in src/backend/utils/adt, > datetime.c:3101:27: warning: use of logical || with constant operand; > switch to bitwise | or remove constant I do *not* see these warnings. Were you using some nondefault compiler option? > (2) clang doesn't support (or require) "-no-cpp-precomp", which > src/template/darwin adds to $CC unconditionally. Adding the flag > unconditionally seems wrong regardless: e.g., -no-cpp-precomp isn't > supported by FSF GCC on OSX either. clang is happy to ignore the flag, > but it just emits countless "warning: argument unused during > compilation: '-no-cpp-precomp'" I do see that, but I also see it complaining about -fwrapv: clang -no-cpp-precomp -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -I../../src/port -DFRONTEND -I../../src/include -c -o chklocale.o chklocale.c clang: warning: argument unused during compilation: '-no-cpp-precomp' clang: warning: argument unused during compilation: '-fwrapv' clang -no-cpp-precomp -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -I../../src/port -DFRONTEND -I../../src/include -c -o dirmod.o dirmod.c clang: warning: argument unused during compilation: '-no-cpp-precomp' clang: warning: argument unused during compilation: '-fwrapv' We're certainly not going to just drop -fwrapv, as that would break the code on many modern versions of gcc. (I'm a bit surprised and concerned that clang hasn't got this flag, btw.) So it would seem that what's needed here is a configure test, not just supplying or not supplying the flag based on environment. > (3) There are countless warnings emitted during the compilation of > regcomp.c and related files, due to unused values returned by ERR(), > VERR(), FAILW(), and similar macros. I fixed this in HEAD, or at least my copy of clang doesn't complain anymore. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 2 Aug 2010 00:40 Neil Conway <neil.conway(a)gmail.com> writes: > (As an aside, is "no-cpp-precomp" still necessary for > reasonably-modern versions of Apple GCC?) I looked into this point a little bit. Apple abandoned their nonstandard precompiler as of gcc 3.3, so the switch is a no-op in that version and later, as per release notes here: http://developer.apple.com/legacy/mac/library/releasenotes/DeveloperTools/RN-GCC3/index.html I have a copy of gcc-3.3 still in captivity, and have verified that it builds 9.0beta4 just fine without the no-cpp-precomp switch. So the question is whether anybody is likely to still be using even-older versions of Apple's gcc. I think probably not: as of Xcode 2.0, released in early 2005, 3.3 was already the oldest supported branch AFAICT. If you'd like to build code that will run on Intel Macs, you have to be using gcc 4.0 or later, so even 3.3 is pretty much in the dustbin of history. So I think we might as well drop -no-cpp-precomp. If there is anyone out there who really needs it, they can always push it in via a manual setting of CPPFLAGS, but there seems little reason to include it by default. I'm still wondering about the bleats I saw for -fwrapv though. configure already is set up to install that switch only conditionally: # Disable optimizations that assume no overflow; needed for gcc 4.3+ PGAC_PROG_CC_CFLAGS_OPT([-fwrapv]) but apparently the test used for this does not notice warning messages. Can we improve that? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Neil Conway on 2 Aug 2010 00:54
On Sun, Aug 1, 2010 at 7:40 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > I tried to duplicate your results using what I believe to be the latest > version of clang, I'm using SVN tip of llvm+clang from ~one week ago. >> (2) clang doesn't support (or require) "-no-cpp-precomp", which >> src/template/darwin adds to $CC unconditionally. Adding the flag >> unconditionally seems wrong regardless: e.g., -no-cpp-precomp isn't >> supported by FSF GCC on OSX either. clang is happy to ignore the flag, >> but it just emits countless "warning: argument unused during >> compilation: '-no-cpp-precomp'" > > I do see that, but I also see it complaining about -fwrapv: > [...] > > We're certainly not going to just drop -fwrapv, as that would break the > code on many modern versions of gcc. �(I'm a bit surprised and concerned > that clang hasn't got this flag, btw.) Support for -fwrapv was apparently implemented recently: https://llvm.org/viewvc/llvm-project?view=rev&sortby=date&revision=106956 FWIW, I think we should aim to eventually remove the dependency on -fwrapv, and instead make the code correct under the semantics guaranteed by the C spec. That will be hard to do without a tool that checks for code that makes incorrect assumptions about integer overflow behavior, but there seems to be progress in that area recently: http://blog.regehr.org/archives/226 (As it happens, their checker uses llvm, which is what motivated me to start poking around in the first place...) >> (3) There are countless warnings emitted during the compilation of >> regcomp.c and related files, due to unused values returned by ERR(), >> VERR(), FAILW(), and similar macros. > > I fixed this in HEAD, or at least my copy of clang doesn't complain > anymore. Yep, looks good here. Thanks! Neil -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |