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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
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

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);



To: mharbison72, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel