Skip to content

[mypyc] declare Py_ssize_t i outside of loop initialization #10091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

thomasjohns
Copy link
Contributor

Description

Fixes mypyc/mypyc#800

I believe this is the only instance of a variable declaration inside a for-loop initialization. Removing this may help with consistent compilation across different C compiler configurations. See linked issue for more details.

Copy link
Collaborator

@TH3CHARLie TH3CHARLie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding this and fixing it!

I'd say there should be some more systematic way to fix this but this definitely helps. Can you confirm the error reported in your issue getting solved with this PR?(I cannot reproduce on my machine).

@thomasjohns
Copy link
Contributor Author

Hi @TH3CHARLie thanks for your question. Yes for me with latest master, any program will not compile when running scripts/mypyc program.py with:

/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt/getargsfast.c:99:9: error: ‘for’ loop initial declarations are only allowed in C99 mode
         for (Py_ssize_t i = 0; i < nargs; i++) {
         ^
/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt/getargsfast.c:99:9: note: use option -std=c99 or -std=gnu99 to compile your code

However, with this patch mypyc compatible programs do compile with scripts/mypyc program.py since the previous "error: 'for' loop initial decl..." isn't raised.


After a bit more experimentation, I found that I get this compiler error with gcc version 4.8.5 and I don't get this compiler error on latest master with gcc version 7.5.0 and clang version 3.4. So this was all likely due to me having an old version of gcc on my system.

One way to solve this generally (if we want to be able to use C code such as for (int i = 0; ...) might be to add certain flags to mypyc/build.py::mypycify based on detecting an old version of gcc (e.g. adding -std=c99 flag will compile with all gcc's and clang's mentioned above).


The following is a little noisy, but it shows what happens when I try to compile build/__native.c with various C compilers on my system (with latest master)

/m/c/U/t/C/mypy> /usr/bin/gcc --version
gcc (Ubuntu 4.8.5-4ubuntu8~14.04.2) 4.8.5
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

/m/c/U/t/C/mypy> /usr/bin/gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt -I/mnt/c/Users/thoma/Code/mypy/.venv/include -I/usr/local/include/python3.9 -c build/__native.c -o build/temp.linux-x86_64-3.9/build/__native.o -O3 -Werror -Wno-unused-function -Wno-unused-label -Wno-unreachable-code -Wno-unused-variable -Wno-unused-command-line-argument -Wno-unknown-warning-option -Wno-unused-but-set-variable
In file included from build/__native.c:3:0:
/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt/getargsfast.c: In function ‘CPyArg_ParseStackAndKeywordsSimple’:
/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt/getargsfast.c:99:9: error: ‘for’ loop initial declarations are only allowed in C99 mode
         for (Py_ssize_t i = 0; i < nargs; i++) {
         ^
/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt/getargsfast.c:99:9: note: use option -std=c99 or -std=gnu99 to compile your code
build/__native.c: At top level:
cc1: error: unrecognized command line option "-Wno-unknown-warning-option" [-Werror]
cc1: error: unrecognized command line option "-Wno-unused-command-line-argument" [-Werror]
cc1: all warnings being treated as errors

(^ error)

/m/c/U/t/C/mypy> gcc-7 --version
gcc-7 (Ubuntu 7.5.0-3ubuntu1~14.04.1) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

/m/c/U/t/C/mypy> gcc-7 -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt -I/mnt/c/Users/thoma/Code/mypy/.venv/include -I/usr/local/include/python3.9 -c build/__native.c -o build/temp.linux-x86_64-3.9/build/__native.o -O3 -Werror -Wno-unused-function -Wno-unused-label -Wno-unreachable-code -Wno-unused-variable -Wno-unused-command-line-argument -Wno-unknown-warning-option -Wno-unused-but-set-variable

(^ success)

/m/c/U/t/C/mypy> clang --version
Ubuntu clang version 3.4-1ubuntu3 (tags/RELEASE_34/final) (based on LLVM 3.4)
Target: x86_64-pc-linux-gnu
Thread model: posix
/m/c/U/t/C/mypy> clang -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/mnt/c/Users/thoma/Code/mypy/mypyc/lib-rt -I/mnt/c/Users/thoma/Code/mypy/.venv/include -I/usr/local/include/python3.9 -c build/__native.c -o build/temp.linux-x86_64-3.9/build/__native.o -O3 -Werror -Wno-unused-function -Wno-unused-label -Wno-unreachable-code -Wno-unused-variable -Wno-unused-command-line-argument -Wno-unknown-warning-option -Wno-unused-but-set-variable

(^ success)

@TH3CHARLie
Copy link
Collaborator

TH3CHARLie commented Feb 16, 2021

One way to solve this generally (if we want to be able to use C code such as for (int i = 0; ...) might be to add certain flags to mypyc/build.py::mypycify based on detecting an old version of gcc (e.g. adding -std=c99 flag will compile with all gcc's and clang's mentioned above).

That's similar to the rough idea in my mind to solve this more systematically. Another potential(but more aggressive) alternative is to reject lower-version C compilers and document this as desired behavior.

I am merging this now. Feel free to comment below if anyone has different opinions on this

@TH3CHARLie TH3CHARLie merged commit dc4f0af into python:master Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C for-loop initial variable declaration may cause compilation error
2 participants