[Rd] Breaking Change in Rcomplex Layout?

[Tomas Kalibera]

On 4/3/23 15:50, Michael Milton wrote:
> Okay, but I'm afraid this will only mean something to Rust users. The 
> reason being that we encountered this issue in extendr: a Rust 
> extension library for R. The specific compiler errors we encounter 
> happen because bindgen (the Rust code generation library) read the 
> changed R header files, and generated a new type definition for 
> Rcomplex. Then, our downstream code that uses that bindgen-generated 
> code caused rustc compiler errors such as:
> |error[E0560]: union `libR_sys::Rcomplex` has no field named `r` --> 
> extendr-api\src\robj\into_robj.rs:93:20 | 93 | Rcomplex { r: 0., i: 0. 
> } | ^ `libR_sys::Rcomplex` does not have this field | = note: 
> available fields are: `__bindgen_anon_1`, `private_data_c`|
> and
> |error[E0609]: no field `i` on type `libR_sys::Rcomplex` --> 
> extendr-api\src\scalar\rcplx_default.rs:105:35 | 105 | 
> Rcplx(c64::new(val.r, val.i)) | ^ unknown field | = note: available 
> fields are: `__bindgen_anon_1`, `private_data_c` help: one of the 
> expressions' fields has a field of the same name | 105 | 
> Rcplx(c64::new(val.r, val.__bindgen_anon_1.i)) | +++++++++++++++++|
That seems like the generator does not support anonymous structures. 
Thanks for letting me know, but I am afraid it is not something that 
could be fixed on the R end.
> However, to put this into context, I would expect that C, C++ packages 
> would encounter a similar issue if they tried to access or modify 
> specific Rcomplex fields in the same way.

No, you can access the fields as before in C (C11):

Rcomplex z;

z.r = 1; z.i = 2

this is what anonymous structures allow, this is why the structure is 
anonymous, not to break existing code.  The union there is to tell the 
compiler about the aliasing, to prevent it from misoptimizing the code. 
As far as I can tell this is valid C code, not a breaking change.

The only issues I know about so far are that you get a warning for 
initializations such as "z = {1, 2}", but still, they will be 
interpreted correctly by the compiler within the standard. However, "z = 
{ .r = 1, .i = 2}" worked before and works with the change.

The definition unfortunately is not valid C++, but works with C++ 
language extensions supported at least by GCC and clang, yet you may see 
warnings. R itself doesn't use C++, but some packages do.

Please note your subject is not quite right: the layout of Rcomplex did 
not change, at least not on systems supported by R. The layout is the 
same on systems which use self-alignment of structures.

Also please keep in mind R-devel is meant for unstable changes, it may 
not work, things may be reverted, it is partially used for testing. I've 
still tested this on all CRAN and Bioconductor packages before 
committing even to R-devel, but this is not always done and cannot be 
due to how much resources it takes. Obviously this will get a NEWS entry 
when/if it gets ported to 4.3.0 branch, in the form it would have at 
that point.

Best
Tomas


>
> On Mon, Apr 3, 2023 at 11:39 PM Tomas Kalibera 
> <tomas.kalibera using gmail.com> wrote:
>
>
>     On 4/3/23 14:07, Michael Milton wrote:
>     > Hi all,
>     >
>     > There seems to have been a breaking change in the R trunk caused
>     by a fix
>     > to bug 18430 <https://bugs.r-project.org/show_bug.cgi?id=18430> that
>     > relates to the layout of the Rcomplex typedef. Previously it was
>     a struct,
>     > but now it's a union by default
>     >
>     <https://github.com/r-devel/r-svn/commit/862f9f816ff3ff3cb3f851603f19e99f60a98475#diff-e9b09a44d9dc69444eca2300325e790a0cc6d2c8c3960f47519c7f8ef896f9e4>,
>     > which breaks downstream code that relied on this layout. I'm
>     aware of
>     > the R_LEGACY_RCOMPLEX variable, but I still wouldn't expect an
>     unreported
>     > breaking change especially if it's aimed at R 4.3 (although I'm
>     not sure if
>     > it is). I believe src/include/R_ext/Complex.h, which this patch
>     affects, is
>     > considered part of the public R ABI since it's included by R.h.
>     >
>     > What should I, as a downstream package developer, do about this
>     change?
>
>     Please report the actual problem you have ran into.
>
>     Thanks
>     Tomas
>
>     >
>     > Cheers.
>     >
>     >       [[alternative HTML version deleted]]
>     >
>     > ______________________________________________
>     > R-devel using r-project.org mailing list
>     > https://stat.ethz.ch/mailman/listinfo/r-devel
>
	[[alternative HTML version deleted]]