This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: ISO C cleanup
Type: compile error Stage: resolved
Components: None Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, eric.araujo, ezio.melotti, gregory.p.smith, hfuru, orsenthil, pitrou, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2010-11-08 14:11 by hfuru, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
iso-c.zip hfuru, 2010-11-08 14:11 ISO C patches
iso_c_32.patch vstinner, 2010-11-09 09:44 review
Messages (20)
msg120742 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-08 14:11
Here are some ISO C conformance patches, and
a minor cleanup I encountered along the way.

Lib/distutils/tests/test_config_cmd.py // comment --> /* comment */.
Lib/distutils/tests/test_build_ext.py,
Objects/weakrefobject.c,
Modules/_pickle.c,
Modules/_testcapimodule.c,
Python/import.c  .  .  .  .  .  .  .   Remove some invalid ',' and ';'s.
Python/Python-ast.c,
Modules/arraymodule.c  .  .  .  .  .   Non-constant array initializer.
Modules/_csv.c   .  .  .  .  .  .  .   Slight cleanup.
Modules/_ctypes/libffi/src/x86/ffi.c   Empty translation unit.

TODO when bored, if anyone cares for more pedantic ISO patches:
- printf(%p, non-void* pointer): The pointer should be cast to void*.
- More // comments -> /**/, but mostly in system-specific code so I
  can't test any patches I make.
msg120747 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-08 14:57
If it’s not too much trouble for you, please post diffs as text files rather than binary.  You can also use “svn diff” to produce one file with all differences.
msg120748 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-08 14:59
By the way, do these changes actually fix errors or are they just cleanups and pedantic (not a slight) fixes?  If the latter, I think they won’t go into stable branches.  I don’t do C though, so I won’t be the one to judge.  Thanks for the patches!
msg120846 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-09 08:45
,AIric Araujo writes:
> By the way, do these changes actually fix errors or are they just
> cleanups and pedantic (not a slight) fixes?

I've used compilers where they'd be compile errors, though I found them
just with gcc -pedantic in an idle moment.

> If the latter, I think they won�t go into stable branches.

Not until someone has better time anyway...

About the .zip, it's 3 files (diff against python 3, 2, and both), I'll
upload them as an ascii file with '#####'s between when bugs.python.org
responds again.
msg120848 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-11-09 08:54
Hallvard, if it going be a rather huge diff, you may make it available at http://bugs.python.org/review, so that it is easy for review.
msg120850 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-11-09 09:44
I commited a part of your patches:

r86353 Remove ";" after function definition, invalid in ISO C
r86354: [array] int => Py_UNICODE (my commit is a little bit different, but it is based on your patch)
r86355: [_pickle] Remove useless comma, invalid in ISO C
r86356: [_csv] Remove useless (duplicate) initialization

I attach the uncommit part of your patches for 3.2.

--

For libffi, the project is maintained outside Python: you should also post your patch to http://sourceware.org/libffi/

Python-ast.c: why do you move req_name and req_type outside PyAST_obj2mod()?
msg120851 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-09 09:54
STINNER Victor writes:
> Python-ast.c: why do you move req_name and req_type outside PyAST_obj2mod()?

Because there's no need to initialize the arrays each time PyAST_obj2mod
is called.  C90-friendly code inside PyAST_obj2mod would be

PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode)
{
    mod_ty res;
    static char *const req_name[] = {"Module", "Expression", "Interactive"};
    PyObject *req_type[];
    req_type[0] = (PyObject*)Module_type;
    req_type[1] = (PyObject*)Expression_type;
    req_type[2] = (PyObject*)Interactive_type;
    ...
}

which is what the current code actually executes.

(I moved req_name to keep it next to req_type in the code.)
msg120852 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-09 09:56
Hallvard B Furuseth writes:
> (...) which is what the current code actually executes.

Er, I mean, that's what it does with req_type.
The 'static' for req_name is an optimization of the current code.
msg120861 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-09 11:46
> I've used compilers where they'd be compile errors, though I found
> them just with gcc -pedantic in an idle moment.

If Victor accepts the patches then they’re useful :)

>> If the latter, I think they won’t go into stable branches.
> Not until someone has better time anyway...

It’s not a question of time but policy: Stable branches (2.7 and 3.1) only get bug fixes, not new features or code cleanups.

> About the .zip, it's 3 files (diff against python 3, 2, and both),
> I'll upload them as an ascii file with '#####'s between

It’s actually easier: “svn diff file1 dir/file2 dir/file3” produces one valid diff.
msg121032 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-12 10:17
STINNER Victor writes:
> Python-ast.c: why do you move req_name and req_type outside PyAST_obj2mod()?

Eh, in case I've managed to be sufficiently unclear: The reason I
modified it at all was because the initialization is not valid in C90,
only in C99.
msg121046 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-12 17:06
> Stable branches (2.7 and 3.1) only get bug fixes, not new features or
> code cleanups.

Éric, making code compliant *is* a bug fix (admittedly of minor importance here, since all modern compilers accept the non-compliant code anyway).
msg137191 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-05-29 16:14
New changeset 74fa7b4b934f by Éric Araujo in branch 'default':
Port r86353 to packaging (#10359: “;” after function definition is invalid in ISO C)
http://hg.python.org/cpython/rev/74fa7b4b934f
msg145099 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-10-07 16:29
According to the latest patch updated by Victor, there are only 3 files left:
 * Lib/distutils/tests/test_config_cmd.py
 * Python/Python-ast.c
 * Modules/_ctypes/libffi/src/x86/ffi.c

The first is in a test and it's probably safe to fix (Éric, do you want to take care of that?) and the last should be reported upstream.
About the one in Python/Python-ast.c, either we fix it or if the proposed patch is not acceptable, we can close this issue as soon as the first file is fixed.
msg145104 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-07 17:09
I will commit this fix and push this week-end or Monday.
msg145216 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-10-09 06:59
New changeset fe0972e102cd by Éric Araujo in branch '3.2':
Make C code in one distutils test comply with ISO C (#10359).
http://hg.python.org/cpython/rev/fe0972e102cd

New changeset 9ded1f21f0fd by Éric Araujo in branch 'default':
Make C code in one packaging test comply with ISO C (#10359).
http://hg.python.org/cpython/rev/9ded1f21f0fd
msg145225 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-10-09 07:00
New changeset 134b68cae802 by Éric Araujo in branch '2.7':
Make C code in one distutils test comply with ISO C (#10359).
http://hg.python.org/cpython/rev/134b68cae802
msg148247 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-24 13:46
Do the changes to Python/Python-ast.c and Modules/_ctypes/libffi/src/x86/ffi.c still apply?  (libffi is an external project, but our copy is already edited so we might as well do one more change).
msg185004 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2013-03-22 21:00
It looks like someone has already done the Python-ast cleanup and I don't understand the point of the distutils test one or the libffi one (ffi has been updated but it's trivial and would still apply).
msg185007 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2013-03-22 21:02
I'm not inclined to apply the patch to ffi unless someone can demonstrate that it is an actual problem.  given that nobody has applied it to *upstream* libffi (since we just pulled in v3.0.13 earlier this week) I doubt it matters to anyone.

everything here looks complete.
msg185012 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-03-22 21:54
> I don't understand the point of the distutils test one

Making the test not fail wrongly with some compilers.  This was committed.
History
Date User Action Args
2022-04-11 14:57:08adminsetgithub: 54568
2013-03-22 21:54:20eric.araujosetmessages: + msg185012
stage: patch review -> resolved
2013-03-22 21:02:57gregory.p.smithsetstatus: open -> closed
resolution: fixed
messages: + msg185007
2013-03-22 21:00:10gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg185004
2011-11-24 13:46:23eric.araujosetmessages: + msg148247
2011-10-09 07:00:14python-devsetmessages: + msg145225
2011-10-09 06:59:47python-devsetmessages: + msg145216
2011-10-07 17:09:00eric.araujosetmessages: + msg145104
2011-10-07 16:29:09ezio.melottisetversions: + Python 3.3, - Python 3.1
nosy: + ezio.melotti

messages: + msg145099

stage: patch review
2011-05-29 16:14:19python-devsetnosy: + python-dev
messages: + msg137191
2010-11-12 17:06:47pitrousetnosy: + pitrou
messages: + msg121046
2010-11-12 10:17:55hfurusetmessages: + msg121032
2010-11-09 11:46:18eric.araujosetmessages: + msg120861
2010-11-09 09:56:23hfurusetmessages: + msg120852
2010-11-09 09:54:43hfurusetmessages: + msg120851
2010-11-09 09:44:57vstinnersetfiles: + iso_c_32.patch

nosy: + vstinner
messages: + msg120850

keywords: + patch
2010-11-09 08:54:53orsenthilsetnosy: + orsenthil
messages: + msg120848
2010-11-09 08:45:33hfurusetmessages: + msg120846
2010-11-08 15:00:57pitrousetnosy: + amaury.forgeotdarc
2010-11-08 14:59:53eric.araujosetmessages: + msg120748
2010-11-08 14:57:56eric.araujosetnosy: + eric.araujo
messages: + msg120747
2010-11-08 14:11:31hfurucreate