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

login
register
mail settings
Submitter phabricator
Date March 20, 2020, 2:06 p.m.
Message ID <differential-rev-PHID-DREV-xxrunyai7dylrerchiut-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45838/
State Superseded
Headers show

Comments

phabricator - March 20, 2020, 2:06 p.m.
mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

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

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: mercurial-devel
phabricator - March 20, 2020, 2:07 p.m.
mharbison72 added a comment.


  This is needed on stable to fix the build.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers
Cc: mercurial-devel
phabricator - March 20, 2020, 2:15 p.m.
marmoute added a comment.
marmoute accepted this revision.


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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
Yuya Nishihara - March 20, 2020, 2:34 p.m.
>   Could we enable some kind of warning to catch this in the tests ?

-Wdeclaration-after-statement for gcc and maybe clang.
phabricator - March 20, 2020, 2:36 p.m.
mharbison72 added a comment.


  In D8304#124109 <https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
phabricator - March 20, 2020, 2:40 p.m.
yuja added a comment.


  >   Could we enable some kind of warning to catch this in the tests ?
  
  -Wdeclaration-after-statement for gcc and maybe clang.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
phabricator - March 20, 2020, 2:54 p.m.
marmoute added a comment.


  In D8304#124116 <https://phab.mercurial-scm.org/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 ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
phabricator - March 20, 2020, 3:09 p.m.
mharbison72 added a comment.


  In D8304#124116 <https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
Yuya Nishihara - March 20, 2020, 3:17 p.m.
>   > -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.
phabricator - March 20, 2020, 3:18 p.m.
yuja added a comment.


  >   > -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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
phabricator - March 20, 2020, 3:19 p.m.
marmoute added a comment.


  Great, @mharbison72 can you submit a patch ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
phabricator - March 21, 2020, 3:56 a.m.
mharbison72 added a comment.


  In D8304#124121 <https://phab.mercurial-scm.org/D8304#124121>, @marmoute wrote:
  
  > Great, @mharbison72 can you submit a patch ?
  
  D8318 <https://phab.mercurial-scm.org/D8318>

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -153,11 +153,12 @@ 
 {
 	if (self->inlined && pos > 0) {
 		if (self->offsets == NULL) {
+			Py_ssize_t ret;
 			self->offsets = PyMem_Malloc(self->raw_length *
 			                             sizeof(*self->offsets));
 			if (self->offsets == NULL)
 				return (const char *)PyErr_NoMemory();
-			Py_ssize_t ret = inline_scan(self, self->offsets);
+			ret = inline_scan(self, self->offsets);
 			if (ret == -1) {
 				return NULL;
 			};