8000 TST: Start testing with "-std=c99" on travisCI. by charris · Pull Request #11905 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content
8000

TST: Start testing with "-std=c99" on travisCI. #11905

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
merged 1 commit into from
Sep 24, 2018

Conversation

charris
Copy link
Member
@charris charris commented Sep 7, 2018

C99 looks to work with gcc for all the Python versions we currently
support. Testing with c99 is a step forward in testing #11888 before
it gets merged after we drop Python 2.7 support.

[skip appveyor]
C99 looks to work with gcc for all the Python versions we currently
support. Testing with c99 is a step forward in testing numpy#11888 before
it gets merged after we drop Python 2.7 support.
@charris charris changed the title WIP, TST: Experiment with c99 TST: Start testing with "-std=c99" on travisCI. Sep 7, 2018
@eric-wieser
Copy link
Member

Would this eventually end up as a change within distutils?

@charris
Copy link
Member Author
charris commented Sep 8, 2018

Would this eventually end up as a change within distutils?

Maybe. Given that some downstream projects use numpy distutils, we may want to add c99 compilers, such as linux does.

charris@fc [numpy.git (master)]$ c99 -v
...
gcc version 8.1.1 20180712 (Red Hat 8.1.1-5) (GCC) 

@charris
Copy link
Member Author
charris commented Sep 8, 2018

Maybe this is a better way of showing what linux does

charris@fc [numpy.git (master)]$ cat  `which c99`
#!/bin/sh
fl="-std=c99"
for opt; do
  case "$opt" in
    -std=c99|-std=iso9899:1999) fl="";;
    -std=*) echo "`basename $0` called with non ISO C99 option $opt" >&2
	    exit 1;;
  esac
done
exec gcc $fl ${1+"$@"}

@charris
Copy link
Member Author
charris commented Sep 8, 2018

@pv @rgommers @juliantaylor Thoughts on C99?

@rgommers
Copy link
Member
rgommers commented Sep 8, 2018

+1 for full C99 compat, but I don't think we can add it to numpy.distutils as a default flag - that will break too many other packages. New compilers doesn't sound attractive; I'd just add a flag to setup.py somewhere.

@pv
Copy link
Member
pv commented Sep 8, 2018

I'm not sure any changes in numpy.distutils are required --- there's potential downstream breakage, and those who need this sort of flags or compiler executables can add them themselves (similarly as here).

For numpy itself, C99 should be fine. That MSVC doesn't support it fully is ridiculous, but IIUC the newer versions Python3 uses are better than the 2.7 situation.

@rgommers
Copy link
Member
rgommers commented Sep 8, 2018

That MSVC doesn't support it fully is ridiculous, but IIUC the newer versions Python3 uses are better than the 2.7 situation.

Yeah, Steve Dower said that anything in C99 not supported for MSVC 2015 (IIRC, could be 2017) is considered a bug. So should be fine by now for Python 3.x

@charris
Copy link
Member Author
charris commented Sep 8, 2018

anything in C99 not supported for MSVC 2015 (IIRC, could be 2017)

The bare restrict keyword does not seem supported, although there is a similar __restrict.

@charris
Copy link
Member Author
charris commented Sep 8, 2018

Note that pocketfft is now building on both travis and appveyor, #11888.

@charris charris 9E85 added this to the 1.17.0 release milestone Sep 22, 2018
@charris
Copy link
Member Author
charris commented Sep 24, 2018

Going to merge this. The -Wno-declaration-after-statement flag is still there, as that also fails for Python 2.7 on Windows and gcc gives a much nicer error message. We can remove that when we drop Python 2.7 and add the flag somewhere else for building NumPy. Do we have any other compiler specific stuff in the build process that isn't part of distutils?

@charris charris merged commit e9d36e4 into numpy:master Sep 24, 2018
@charris charris deleted the c99-tests-update branch October 12, 2018 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0