Opened 2 years ago

Closed 2 years ago

#23789 closed defect (fixed)

Drop -std=gnu99 from $CC

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-8.1
Component: build: configure Keywords:
Cc: mkoeppe, fbissey Merged in:
Authors: François Bissey Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 4acda8c (Commits) Commit: 4acda8ce4933a24e02171754806a6d6d2cd08875
Dependencies: Stopgaps:

Description

This is bad:

$ env |grep 99
CONFIGURED_CC=gcc -std=gnu99
CC=gcc -std=gnu99

I did not ask for -std=gnu99 and configure should not unilaterally decide that I want that.

In particular, there is a long-standing Python bug causing distutils to use $CC to compile C++ code leading to plenty of warnings

cc1plus: warning: command line option ‘-std=gnu99’ is valid for C/ObjC but not for C++

If I recall correctly, the analogous behaviour for clang is a hard error instead of a warning. So this might break a clang build.

Change History (16)

comment:1 follow-up: Changed 2 years ago by fbissey

That should only happen if we use AC_PROG_CC_C99 in configure.ac.

comment:2 in reply to: ↑ 1 Changed 2 years ago by jdemeyer

Replying to fbissey:

That should only happen if we use AC_PROG_CC_C99 in configure.ac.

...which we do in order to check that C99 is supported:

if test -z "$CC"; then
    SAGE_MUST_INSTALL_GCC([a C compiler is missing])
else
    # Check that we can compile C99 code
    AC_PROG_CC_C99()
    if test "x$ac_cv_prog_cc_c99" = xno
    then
        SAGE_MUST_INSTALL_GCC([your C compiler cannot compile C99 code])
    fi
fi

comment:3 follow-up: Changed 2 years ago by fbissey

Right! If we don't want to default to C99 then we have to be more careful and save the compiler before testing C99 and then reset it since that macro will change CC. I guess this is only troublesome for older gcc (4.x I think) which don't default to C99 without a flag. clang and recent gcc default to at least that or better, so no flag is needed or added. That must say something on who's testing :)

Last edited 2 years ago by fbissey (previous) (diff)

comment:4 follow-up: Changed 2 years ago by jdemeyer

The easiest solution is probably to save the value of $CC before the test and then restore it.

comment:5 in reply to: ↑ 4 Changed 2 years ago by fbissey

Replying to jdemeyer:

The easiest solution is probably to save the value of $CC before the test and then restore it.

+1

comment:6 in reply to: ↑ 3 Changed 2 years ago by jdemeyer

Replying to fbissey:

I guess this is only troublesome for older gcc (4.x I think) which don't default to C99 without a flag.

I'm using

gcc (Gentoo 4.9.4 p1.0, pie-0.6.4) 4.9.4
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.

comment:7 Changed 2 years ago by jdemeyer

I'm currently working on some other things. Feel free to implement this, otherwise I'll do it later.

comment:8 follow-up: Changed 2 years ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/no_c99
  • Commit set to f34394d84ed07c4ed492479136fed334abd14b19

I do not have the compiler to test this. It will probably need its own configure tarball. But some testing first.

comment:9 in reply to: ↑ 8 Changed 2 years ago by jdemeyer

Replying to fbissey:

It will probably need its own configure tarball.

No, this is just a minor bug fix.

comment:10 Changed 2 years ago by git

  • Commit changed from f34394d84ed07c4ed492479136fed334abd14b19 to 634444ae4a2575153e5e7a234f702f862f3649ca

Branch pushed to git repo; I updated commit sha1. New commits:

634444asave the C compiler before checking for C99 availability and then restoring it.

comment:11 Changed 2 years ago by fbissey

  • Status changed from new to needs_review

Now with a commit.

Last edited 2 years ago by fbissey (previous) (diff)

comment:12 Changed 2 years ago by jdemeyer

Typo: cheking

comment:13 Changed 2 years ago by jdemeyer

  • Branch changed from u/fbissey/no_c99 to u/jdemeyer/no_c99

comment:14 Changed 2 years ago by jdemeyer

  • Commit changed from 634444ae4a2575153e5e7a234f702f862f3649ca to 4acda8ce4933a24e02171754806a6d6d2cd08875
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

New commits:

4acda8cTypo

comment:15 Changed 2 years ago by fbissey

Thanks for taking over. I was busy with "bedtime stories" for my son :)

comment:16 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/no_c99 to 4acda8ce4933a24e02171754806a6d6d2cd08875
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.