Skip to content
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

CMake: CMAKE_C_EXTENSIONS=ON seems mandatory (with clang) #2

Closed
Mizux opened this issue Jul 29, 2021 · 4 comments
Closed

CMake: CMAKE_C_EXTENSIONS=ON seems mandatory (with clang) #2

Mizux opened this issue Jul 29, 2021 · 4 comments
Labels

Comments

@Mizux
Copy link

Mizux commented Jul 29, 2021

TLDR: It seems the SCIP source depends on M_PI which is a GNU extension, i.e. compiling using -std=c99 won't work.

Protocol

If here:

scip/src/CMakeLists.txt

Lines 10 to 12 in a6142b7

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED on)

I'm adding the line

set(CMAKE_C_EXTENSIONS OFF)

Then CMake will generate compile line using -std=c99 instead of the "default" -std=gnu99

ref: https://cmake.org/cmake/help/latest/variable/CMAKE_CXX_EXTENSIONS.html
note: You can also pass it on CMake configure line or could be given by a super project integrating SCIP from sources...
note2: Usually CMake default enable extensions seems to have change 17months ago see
Kitware/CMake@f034b0f

Observed

note: I'm doing some integration tests of multithread SCIP so technically I building SCIP using through FetchContent()
full project here: https://github.com/Mizux/scip-multithread

cmake --build build -v
...
[ 17%] Building C object _deps/scip-build/src/CMakeFiles/libscip.dir/scip/cons_soc.c.o
cd /home/mizux/dev/scip-multithread/build/_deps/scip-build/src && /usr/bin/clang
 -I/home/mizux/dev/scip-multithread/build/_deps/scip-build
 -I/home/mizux/dev/scip-multithread/build/_deps/scip-src/src
 -I/home/mizux/dev/scip-multithread/build/_deps/zlib-build
 -I/home/mizux/dev/scip-multithread/build/_deps/zlib-src
 -fsanitize=thread -g -fPIC -fvisibility=hidden -std=c99 # SEE HERE the use of -std=c99
 -MD -MT
 _deps/scip-build/src/CMakeFiles/libscip.dir/scip/cons_soc.c.o
 -MF CMakeFiles/libscip.dir/scip/cons_soc.c.o.d
 -o CMakeFiles/libscip.dir/scip/cons_soc.c.o
 -c /home/mizux/dev/scip-multithread/build/_deps/scip-src/src/scip/cons_soc.c
/home/mizux/dev/scip-multithread/build/_deps/scip-src/src/scip/cons_soc.c:1996:10: error: use of undeclared identifier 'M_PI'
   val = M_PI;
         ^
/home/mizux/dev/scip-multithread/build/_deps/scip-src/src/scip/cons_soc.c:2238:13: error: use of undeclared identifier 'M_PI'
      val = M_PI / pow(2.0, (double) (i+1));
            ^
/home/mizux/dev/scip-multithread/build/_deps/scip-src/src/scip/cons_soc.c:2313:19: error: use of undeclared identifier 'M_PI'
   vals[0] = tan( M_PI / pow(2.0, (double) (N+1)) );
                  ^
3 errors generated.
make[2]: *** [_deps/scip-build/src/CMakeFiles/libscip.dir/build.make:776: _deps/scip-build/src/CMakeFiles/libscip.dir/scip/cons_soc.c.o] Error 1
make[2]: Leaving directory '/home/mizux/dev/scip-multithread/build'
make[1]: *** [CMakeFiles/Makefile2:1057: _deps/scip-build/src/CMakeFiles/libscip.dir/all] Error 2
make[1]: Leaving directory '/home/mizux/dev/scip-multithread/build'
make: *** [Makefile:166: all] Error 2

note: I tweaked the indent of the trace
fun fact: PI only officially exist since C++20 (https://en.cppreference.com/w/cpp/numeric/constants)

Proposal

If i were you:

  1. I would be explicit, because in case of a super project build integrating scip with this option to OFF it will fail.
    Ie since it seems mandatory you should force it -> set(CMAKE_C_EXTENSIONS ON)
  2. add a define in case...
    #ifndef M_PI
    #define M_PI 3.14159265358979323846
    #endif

Platform

FYI

%uname -a
Linux nuc10i7 5.13.5-arch1-1 #1 SMP PREEMPT Sun, 25 Jul 2021 18:02:58 +0000 x86_64 GNU/Linux
%gcc --version
gcc (GCC) 11.1.0
% clang --version
clang version 12.0.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
@svigerske
Copy link
Member

It is supposed to be -std=c99 -D_XOPEN_SOURCE=600: https://github.com/scipopt/scip/blob/master/make/make.linux.gnu#L12

@Mizux
Copy link
Author

Mizux commented Jul 29, 2021

Not sure this makefile config file will impact the CMake build unless you do some magic...

note: Update the title

@Mizux Mizux changed the title CMAKE_C_EXTENSIONS=ON seems mandatory (with clang) CMake: CMAKE_C_EXTENSIONS=ON seems mandatory (with clang) Jul 29, 2021
@svigerske
Copy link
Member

No, it's not. It was meant as a hint to the people who take care of the cmake system, since, as far as I rememeber, the goal is to have the cmake system behave equivalently to the normal makefiles at some point.

@fschloesser
Copy link
Contributor

This should be resolved with 5d5bd88

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants