Opened 6 years ago

Closed 6 years ago

#4116 closed defect (fixed)

Select a C standard and include in CFLAGS

Reported by: dbaston Owned by: pramsey
Priority: medium Milestone: PostGIS 3.0.0
Component: postgis Version: master
Keywords: Cc:

Description

I made some changes that compiled without errors locally, but when I made a PR, Travis complained about declaring a variable inside a for loop, e.g.:

for (int i = 0).

Trying to reproduce this locally, I added -std=c89 to my CFLAGS, which indeed produced this error, but also some new errors about usage of the inline keyword. Switching to -std=gnu89 seemed to reproduce the conditions of the Travis environment.

I'd suggest that we settle on a non-GNU standard (-std=c99 ?) and add it to our build flags so we can get consistent results with different compilers.

Change History (11)

comment:1 by Algunenano, 6 years ago

I definitely like the idea of updating the c standard. I think c89 is being used because it's the default for Postgresql but since some language extensions were used (M_PI is another example) the de facto standard is gnu89.

If we decided to update I'd go with c11 directly, which is fully supported from gcc 4.9+ (and partially before that, https://gcc.gnu.org/wiki/C11Status), released in 2014, but I'd be satisfied with c99 too.

Nevertheless, I wouldn't update it for 2.5 (for the sake of keeping compatibility) but I would for the next one (2.6?, 3.0?).

comment:2 by dbaston, 6 years ago

If nothing else, I guess we could add -std=gnu89 for PostGIS 2.5 ?

comment:3 by komzpa, 6 years ago

Windows is the most limiting factor in the choice.

Regina, what's the newest C we can choose that won't break Windows?

If nothing gets broken I'd say let's go to c11 directly. GNU sounds non-Windowsy.

comment:4 by dbaston, 6 years ago

For PostGIS <= 2.5: I'd rather not use a GNU version either, but since we are, it seems like we might as well specify it in a build flag.

For future versions: Out of curiosity, are there features that seem compelling in c11 ? Though I have no objection to using it, it's not obvious to me what we gain from it.

comment:5 by komzpa, 6 years ago

Two things I can see use of are static_assert for compile time assumption checks (like sizeof(double)==sizeof(int64) for type punning), and anonymous structs/unions (so type-punned double/int64 does not need require inventing a type name for it).

comment:6 by komzpa, 6 years ago

Googling shows MSVC does not support c11, and stopped at implementing most features of C99. As long as MSVC is used, I think we can't go above C99, so let's go there directly. :)

comment:7 by dbaston, 6 years ago

I started a branch for -std=c99 support a few days ago, it's here: https://github.com/dbaston/postgis/commit/65967aa5d4f9dcce1b8f2be71b7796419c8be27f

Compiling liblwegeom under -std=c99 is no big deal, but it looks like GNU extensions are necessary once you start bringing in Postgres code (libpgcommon)

comment:8 by robe, 6 years ago

We don't support MSVC compile. All windows compiles are done using Mingw64 GCC+ compilers.

I think we had the cmake thing going on that supports MSVC, but that's at this point a forked project, see #2362 and I'm not interested in supporting CMake unless someone steps up to the plate to take ownership of it. At this point I don't think active PostGIS dev team members are interested in taking ownership of CMake support.

That may change in future, but at this point any talk of MSVC support in my mind is moot. By that time Microsoft will probably be pushing linux tools anyway since that is their future and windows is their past.

comment:9 by robe, 6 years ago

Milestone: PostGIS 2.5.0PostGIS 3.0.0

comment:10 by dbaston, 6 years ago

Set -std=gnu99 in r16845

comment:11 by dbaston, 6 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.