Page MenuHomePhabricator

cext: move variable declaration to the top of the block for C89 support
ClosedPublic

Authored by mharbison72 on Fri, Mar 20, 10:06 AM.

Details

Summary

Not sure if we still care about C89 in general, but MSVC requires this style
too.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Fri, Mar 20, 10:06 AM

This is needed on stable to fix the build.

marmoute accepted this revision.Fri, Mar 20, 10:15 AM
marmoute added a subscriber: marmoute.

Could we enable some kind of warning to catch this in the tests ?

Could we enable some kind of warning to catch this in the tests ?

I don't think so. You need CFLAGS=-std=c89 (and maybe -pedantic). But in C89 mode with gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0, it fails to build complaining about C++ comments and not understanding inline, and also issues a bunch of warnings about various things.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Fri, Mar 20, 10:40 AM
Could we enable some kind of warning to catch this in the tests ?

-Wdeclaration-after-statement for gcc and maybe clang.

In D8304#124116, @yuja wrote:
Could we enable some kind of warning to catch this in the tests ?

-Wdeclaration-after-statement for gcc and maybe clang.

Nice, any idea of how to make it verbosely complaining part of the test-suite ?

In D8304#124116, @yuja wrote:
Could we enable some kind of warning to catch this in the tests ?

-Wdeclaration-after-statement for gcc and maybe clang.

I can confirm that this works on 18.04, though it doesn't issue a warning (or complain about an unknown option) on 10.14 with gcc/clang.

yuja added a comment.Fri, Mar 20, 11:18 AM
> -Wdeclaration-after-statement for gcc and maybe clang.
Nice, any idea of how to make it verbosely complaining part of the test-suite ?

-Werror=declaration-after-statement will prevent the test from running at all.
I think setup.py can be adjusted to add some extra compiler options.

Great, @mharbison72 can you submit a patch ?

Great, @mharbison72 can you submit a patch ?

D8318