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

[ci] [R-package] test against R 4.3 on Windows #6061

Merged
merged 16 commits into from
Oct 6, 2023
Merged

[ci] [R-package] test against R 4.3 on Windows #6061

merged 16 commits into from
Oct 6, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Aug 24, 2023

Contributes to #3756.

Moves all R 4.x CI jobs to R 4.3.1 (the latest available version).

Issues this PR's changes address

On Windows (minGW)

* checking whether package 'lightgbm' can be installed ... [222s] WARNING

Found the following significant warnings:

  src\external_libs\fmt\include\fmt\core.h:2042:48: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]

See 'D:/a/LightGBM/LightGBM/lightgbm.Rcheck/00install.out' for details.

This did not show up on R 4.2. It seems related to fmtlib/fmt#3533, and seems to have been fixed by updated fmt in #6074.

On Windows (MSVC)

D:\a\LightGBM\LightGBM\RLibrary\R\include\R_ext/Complex.h(81,21): error C3646: 'private_data_c': unknown override specifier (compiling source file C:\Users\runneradmin\RtmpiGO4kB\Rbuild10684dd767b0\lightgbm\src\src\lightgbm_R.cpp)

Looks like this is caused by this change to include/R_ext/Complex.h in R's headers, here: wch/r-source@fb52ac1. And from that commit, seems that the R Core team understood that this was something that not all compilers might support:

/* This definition uses an anonymous structure, which is defined in C11 (but
not C99).  It is, however, supported at least by GCC, clang and icc.  The
private_data_c member should never be used in code, but tells the compiler
about type punning when accessing the .r and .i elements, so is safer to use
when interfacing with Fortran COMPLEX*16 or directly C99 _Complex double
(PR#18430).

This form of static initialization works with both definitions:
Rcomplex z = { .r = 1, .i = 2 }; */

typedef union {
    struct {
	double r;
	double i;
    };
    double _Complex private_data_c;
} Rcomplex;

Seems to be that MSVC does not like anonymous types in structs: https://stackoverflow.com/a/58571593/3986677.

@jameslamb jameslamb changed the title WIP: [ci] [R-package] test against R 4.3 WIP: [ci] [R-package] test against R 4.3 on Windows Sep 4, 2023
@jameslamb jameslamb changed the title WIP: [ci] [R-package] test against R 4.3 on Windows [ci] [R-package] test against R 4.3 on Windows Sep 19, 2023
@jameslamb jameslamb marked this pull request as ready for review September 19, 2023 23:10
@jameslamb
Copy link
Collaborator Author

@guolinke could you please review this when you have time?

@jameslamb
Copy link
Collaborator Author

thank you!

@jameslamb jameslamb merged commit 3c7e38b into master Oct 6, 2023
39 checks passed
Copy link

This pull request 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 Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants