Patchwork D8330: cext: move more variable declarations to the top of the block for C89 support

login
register
mail settings
Submitter phabricator
Date March 25, 2020, 8:34 p.m.
Message ID <differential-rev-PHID-DREV-y5h7sz4sfkjg6zgr3xsi-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45893/
State Superseded
Headers show

Comments

phabricator - March 25, 2020, 8:34 p.m.
mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  These instances aren't compiled on Windows, so they don't matter much.  But they
  do get flagged by `-Werror=declaration-after-statement` in the next patch.
  
  setup: build C extensions with -Werror=declaration-after-statement
  
  MSVC 2008 still needs declarations at the top of the scope.  I added it to the
  3rd party code too in case somebody vendors a new version with a problem-
  they'll get an early warning.  Clang seems to ignore this (at least on 10.14
  with Xcode 10), and gcc 7.4 will error out as desired on Ubuntu 18.04.  Thanks
  to Yuya for remembering the name of the option.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cext/osutil.c
  setup.py

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2020, 8:36 p.m.
mharbison72 added a comment.
mharbison72 abandoned this revision.


  This was a test run of `phabsend --fold` to show what the UI looks like.  It's a fold of D8317 <https://phab.mercurial-scm.org/D8317> and D8318 <https://phab.mercurial-scm.org/D8318> (with the URLs removed).

REPOSITORY
  rHG Mercurial

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

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

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

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1268,6 +1268,12 @@ 
 ]
 common_include_dirs = ['mercurial']
 
+common_cflags = []
+
+# MSVC 2008 still needs declarations at the top of the scope.
+if os.name != 'nt':
+    common_cflags = ['-Werror=declaration-after-statement']
+
 osutil_cflags = []
 osutil_ldflags = []
 
@@ -1441,18 +1447,21 @@ 
         'mercurial.cext.base85',
         ['mercurial/cext/base85.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
         'mercurial.cext.bdiff',
         ['mercurial/bdiff.c', 'mercurial/cext/bdiff.c'] + xdiff_srcs,
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends + ['mercurial/bdiff.h'] + xdiff_headers,
     ),
     Extension(
         'mercurial.cext.mpatch',
         ['mercurial/mpatch.c', 'mercurial/cext/mpatch.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
@@ -1466,6 +1475,7 @@ 
             'mercurial/cext/revlog.c',
         ],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends
         + ['mercurial/cext/charencode.h', 'mercurial/cext/revlog.h',],
     ),
@@ -1473,7 +1483,7 @@ 
         'mercurial.cext.osutil',
         ['mercurial/cext/osutil.c'],
         include_dirs=common_include_dirs,
-        extra_compile_args=osutil_cflags,
+        extra_compile_args=common_cflags + osutil_cflags,
         extra_link_args=osutil_ldflags,
         depends=common_depends,
     ),
@@ -1482,6 +1492,7 @@ 
         [
             'mercurial/thirdparty/zope/interface/_zope_interface_coptimizations.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
         'mercurial.thirdparty.sha1dc',
@@ -1490,9 +1501,12 @@ 
             'mercurial/thirdparty/sha1dc/lib/sha1.c',
             'mercurial/thirdparty/sha1dc/lib/ubc_check.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
-        'hgext.fsmonitor.pywatchman.bser', ['hgext/fsmonitor/pywatchman/bser.c']
+        'hgext.fsmonitor.pywatchman.bser',
+        ['hgext/fsmonitor/pywatchman/bser.c'],
+        extra_compile_args=common_cflags,
     ),
     RustStandaloneExtension(
         'mercurial.rustext', 'hg-cpython', 'librusthg', py3_features='python3'
@@ -1503,11 +1517,11 @@ 
 sys.path.insert(0, 'contrib/python-zstandard')
 import setup_zstd
 
-extmodules.append(
-    setup_zstd.get_c_extension(
-        name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
-    )
+zstd = setup_zstd.get_c_extension(
+    name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
 )
+zstd.extra_compile_args += common_cflags
+extmodules.append(zstd)
 
 try:
     from distutils import cygwinccompiler
diff --git a/mercurial/cext/osutil.c b/mercurial/cext/osutil.c
--- a/mercurial/cext/osutil.c
+++ b/mercurial/cext/osutil.c
@@ -810,9 +810,10 @@ 
 			/* Check the memory we can use. Typically, argv[i] and
 			 * argv[i + 1] are continuous. */
 			for (i = 0; i < argc; ++i) {
+				size_t len;
 				if (argv[i] > argvend || argv[i] < argvstart)
 					break; /* not continuous */
-				size_t len = strlen(argv[i]);
+				len = strlen(argv[i]);
 				argvend = argv[i] + len + 1 /* '\0' */;
 			}
 			if (argvend > argvstart) /* sanity check */
@@ -1170,9 +1171,9 @@ 
 {
 	int sig = 0;
 	int r;
+	sigset_t set;
 	if (!PyArg_ParseTuple(args, "i", &sig))
 		return NULL;
-	sigset_t set;
 	r = sigemptyset(&set);
 	if (r != 0)
 		return PyErr_SetFromErrno(PyExc_OSError);