This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8304

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

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

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHG3122058df7a5: cext: move variable declaration to the top of the block for C89 support
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.Mar 20 2020, 10:06 AM
mharbison72 added a comment.Mar 20 2020, 10:07 AM

This is needed on stable to fix the build.

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

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

mharbison72 added a comment.Mar 20 2020, 10:36 AM
In D8304#124109, @marmoute wrote:

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.

mharbison72 added a commit: rHG3122058df7a5: cext: move variable declaration to the top of the block for C89 support.Mar 20 2020, 10:37 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG3122058df7a5: cext: move variable declaration to the top of the block for C89 support (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Mar 20 2020, 10:40 AM
Could we enable some kind of warning to catch this in the tests ?

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

marmoute added a comment.Mar 20 2020, 10:54 AM
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 ?

mharbison72 added a comment.Mar 20 2020, 11:09 AM
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.Mar 20 2020, 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.

marmoute added a comment.Mar 20 2020, 11:19 AM

Great, @mharbison72 can you submit a patch ?

mharbison72 added a comment.Mar 20 2020, 11:56 PM
In D8304#124121, @marmoute wrote:

Great, @mharbison72 can you submit a patch ?

D8318

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/revlog.c (3 lines)

Diff 20842

mercurial/cext/revlog.c

/* /*
* Return a pointer to the beginning of a RevlogNG record. * Return a pointer to the beginning of a RevlogNG record.
*/ */
static const char *index_deref(indexObject *self, Py_ssize_t pos) static const char *index_deref(indexObject *self, Py_ssize_t pos)
{ {
if (self->inlined && pos > 0) { if (self->inlined && pos > 0) {
if (self->offsets == NULL) { if (self->offsets == NULL) {
Py_ssize_t ret;
self->offsets = PyMem_Malloc(self->raw_length * self->offsets = PyMem_Malloc(self->raw_length *
sizeof(*self->offsets)); sizeof(*self->offsets));
if (self->offsets == NULL) if (self->offsets == NULL)
return (const char *)PyErr_NoMemory(); return (const char *)PyErr_NoMemory();
Py_ssize_t ret = inline_scan(self, self->offsets); ret = inline_scan(self, self->offsets);
if (ret == -1) { if (ret == -1) {
return NULL; return NULL;
}; };
} }
return self->offsets[pos]; return self->offsets[pos];
} }
return (const char *)(self->buf.buf) + pos * v1_hdrsize; return (const char *)(self->buf.buf) + pos * v1_hdrsize;