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

[R-package] gcc warning on CMake-based builds: 'cast between incompatible function types' #4273

Closed
jameslamb opened this issue May 10, 2021 · 6 comments · Fixed by #4274
Closed

Comments

@jameslamb
Copy link
Collaborator

jameslamb commented May 10, 2021

Description

Building the R package with CMake-based installation (https://github.com/microsoft/LightGBM/blob/master/R-package/README.md#install) and gcc (or MINGW on Windows results in the following compiler warnings:

/private/var/folders/xq/wktq4zdx4jd3qdpk34d28m940000gn/T/Rtmp7sAM9p/R.INSTALL71b1ac713c8/lightgbm/src/src/lightgbm_R.cpp:722:41: warning: cast between incompatible function types from 'SEXPREC* (*)(LGBM_SE, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP)' {aka 'SEXPREC* (*)(LGBM_SER*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*, SEXPREC*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  722 |   {"LGBM_BoosterPredictForMat_R"      , (DL_FUNC) &LGBM_BoosterPredictForMat_R      , 11},
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/private/var/folders/xq/wktq4zdx4jd3qdpk34d28m940000gn/T/Rtmp7sAM9p/R.INSTALL71b1ac713c8/lightgbm/src/src/lightgbm_R.cpp:723:41: warning: cast between incompatible function types from 'SEXPREC* (*)(LGBM_SE, SEXP, SEXP, SEXP)' {aka 'SEXPREC* (*)(LGBM_SER*, SEXPREC*, SEXPREC*, SEXPREC*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  723 |   {"LGBM_BoosterSaveModel_R"          , (DL_FUNC) &LGBM_BoosterSaveModel_R          , 4},
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/private/var/folders/xq/wktq4zdx4jd3qdpk34d28m940000gn/T/Rtmp7sAM9p/R.INSTALL71b1ac713c8/lightgbm/src/src/lightgbm_R.cpp:724:41: warning: cast between incompatible function types from 'SEXPREC* (*)(LGBM_SE, SEXP, SEXP)' {aka 'SEXPREC* (*)(LGBM_SER*, SEXPREC*, SEXPREC*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  724 |   {"LGBM_BoosterSaveModelToString_R"  , (DL_FUNC) &LGBM_BoosterSaveModelToString_R  , 3},
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/private/var/folders/xq/wktq4zdx4jd3qdpk34d28m940000gn/T/Rtmp7sAM9p/R.INSTALL71b1ac713c8/lightgbm/src/src/lightgbm_R.cpp:725:41: warning: cast between incompatible function types from 'SEXPREC* (*)(LGBM_SE, SEXP, SEXP)' {aka 'SEXPREC* (*)(LGBM_SER*, SEXPREC*, SEXPREC*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  725 |   {"LGBM_BoosterDumpModel_R"          , (DL_FUNC) &LGBM_BoosterDumpModel_R          , 3},

One such warning is generated for each entry in this list of routines:

static const R_CallMethodDef CallEntries[] = {

Reproducible example

Follow https://github.com/microsoft/LightGBM/blob/master/R-package/README.md#install, using gcc on Mac or Linux, or MinGW on Windows.

This warning does not appear to show up when using MSVC or clang.

This warning does not appear when using the CRAN-based installation method (https://github.com/microsoft/LightGBM/blob/master/R-package/README.md#installing-the-cran-package).

I've observed this warning with gcc 10, on both R 3.6.3 and R 4.0.5.

Additional Comments

@jameslamb jameslamb changed the title [R-package] gcc warning: 'cast between incompatible function types' [R-package] gcc warning on CMake-based builds: 'cast between incompatible function types' May 10, 2021
@jameslamb
Copy link
Collaborator Author

Looking at build logs today, I see the following flags used.

CRAN build

/usr/local/bin/g++-10 \
  -std=gnu++11 \
  -I"/Library/Frameworks/R.framework/Resources/include" \
  -DNDEBUG \
  -I./include \
  -DEIGEN_MPL2_ONLY \
  -DMM_PREFETCH=1 \
  -DMM_MALLOC=1 \
  -DUSE_SOCKET \
  -DLGB_R_BUILD \
  -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk \
  -I/usr/local/include \
  -pthread \
  -fPIC \
  -Wall \
  -g \
  -O2 \
  -c lightgbm_R.cpp \
  -o lightgbm_R.o

CMake-based

I got these by adding a MESSAGE(FATAL_ERROR "CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}") near the end of CMakelists.txt

/usr/local/bin/g++-10 \
    -fopenmp \
    -std=c++11 \
    -pthread \
    -Wextra \
    -Wall
    -Wno-ignored-attributes \
    -Wno-unknown-pragmas \
    -Wno-return-type \
    -O3 \
    -fPIC \
    -funroll-loops

I think the use of -Wextra explains why these warnings only show up in CMake-based builds of the R package.

LightGBM/CMakeLists.txt

Lines 273 to 274 in 08d1ce4

if(UNIX OR MINGW OR CYGWIN)
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -Wextra -Wall -Wno-ignored-attributes -Wno-unknown-pragmas -Wno-return-type")

-Wcast-function-type is included with -Wextra, which is used on CMake-based builds but not CRAN-based builds.

reference: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html


I think that this warning can safely be ignored for the R package specifically. The pattern used in {lightgbm} is exactly the same one used in many other R packages on CRAN, and the extensive unit tests covering the package give me confidence that this isn't causing any issues.

I'll submit a pull request to suppress it.

@david-cortes
Copy link
Contributor

I think the issue here is that the functions are declared as C++ when they should be declared as extern C.

@jameslamb
Copy link
Collaborator Author

the functions are declared as C++

Can you please clarify what leads you to believe that they are missing extern C? These functions are all defined with LIGHTGBM_C_EXPORT like this

LIGHTGBM_C_EXPORT SEXP LGBM_DatasetCreateFromFile_R(

which, if I'm not mistaken, will end up adding extern "C" at compile time

#ifdef __cplusplus
#define LIGHTGBM_EXTERN_C extern "C"
#else
#define LIGHTGBM_EXTERN_C
#endif
#ifdef _MSC_VER
#define LIGHTGBM_EXPORT __declspec(dllexport)
#define LIGHTGBM_C_EXPORT LIGHTGBM_EXTERN_C __declspec(dllexport)
#else
#define LIGHTGBM_EXPORT
#define LIGHTGBM_C_EXPORT LIGHTGBM_EXTERN_C
#endif

What are we missing?

@david-cortes
Copy link
Contributor

My bad, it's not an issue with those then. Although in any case I'd also include the R headers in an an extern C block just in case.

@david-cortes
Copy link
Contributor

Actually, looking at the R headers in the latest version of R, there doesn't seem to be any need to declare them as extern C, and R_USE_C99_IN_CXX is now added automatically with C++11 and higher.

jameslamb added a commit that referenced this issue May 15, 2021
…nd MinGW builds (fixes #4273) (#4274)

* [R-package] suppress Wcast-function-type warning in CMake-based gcc and MinGW builds (fixes #4273)

* suppress warning from CMake side
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants