We have a helper union sal_math_Double (defined in include/sal/mathconf.h) to help low-level access to double parts. In several places, double values are reinterpret_cast to it, to access parts of the IEEE 754 structure. E.g., in sc/source/core/tool/interpr6.cxx: > sal_uInt32 nErr = reinterpret_cast< sal_math_Double * >(&aRes.mfFirst)->nan_parts.fraction_lo; mathconf.h could define a helper function to make it simpler, which could take a reference to double, and return it cast to reference to sal_math_Double, so the above code could be re-written to something like > sal_uInt32 nErr = sal_math_DoubleParts(aRes.mfFirst).nan_parts.fraction_lo; Besides shorter form, the actual benefit is decreased use of unsafe reinterpret_cast, thus providing better type safety. To use references in the C header, of course, the function must be enclosed in the "#if defined __cplusplus" block; or be defined in another helper header. Alternatively, it should take and return pointers (less ideal IMO). So the task is to implement such a function, and replace existing cases of reinterpret_casts of doubles to the struct with use of this function.
I tend to disagree with adding such a function (esp. to include/sal/mathconf.h, which is part of the stable URE interface; esp. with adding it not conditional on LIBO_INTERNAL_ONLY): I think the very few cases that now use reinterpret_cast<sal_math_Double *>(X)->E could just as well use something like sal_math_Double d; d.value = X; d.E instead.
(In reply to Stephan Bergmann from comment #1) > I tend to disagree with adding such a function (esp. to > include/sal/mathconf.h, which is part of the stable URE interface; esp. with > adding it not conditional on LIBO_INTERNAL_ONLY): The "esp". bits seem like "it's OK to do that *if* it is guarded by LIBO_INTERNAL_ONLY, *or if* it's outside of include/sal/mathconf.h"? ;-) It would be nice if you provided a positive set of conditions :-) (I CCed you exactly because you are the expert who can tell if this is a bad idea after all - just please be verbose why it's bad.) > I think the very few > cases that now use > > reinterpret_cast<sal_math_Double *>(X)->E > > could just as well use something like > > sal_math_Double d; > d.value = X; > d.E > > instead. Yes, but this is awkward, and I don't see a benefit in using this form. Do you? :-)
(In reply to Mike Kaganski from comment #2) > (In reply to Stephan Bergmann from comment #1) > > I tend to disagree with adding such a function (esp. to > > include/sal/mathconf.h, which is part of the stable URE interface; esp. with > > adding it not conditional on LIBO_INTERNAL_ONLY): > > The "esp". bits seem like "it's OK to do that *if* it is guarded by > LIBO_INTERNAL_ONLY, *or if* it's outside of include/sal/mathconf.h"? ;-) It > would be nice if you provided a positive set of conditions :-) (I CCed you > exactly because you are the expert who can tell if this is a bad idea after > all - just please be verbose why it's bad.) If it were functionality that would indeed provide widespread benefit, I would consider it OK to either add it as LIBO_INTERNAL_ONLY to a stable URE interface file, or to a file outside the stable URE interface (e.g., somewhere in include/o3tl/). > > I think the very few > > cases that now use > > > > reinterpret_cast<sal_math_Double *>(X)->E > > > > could just as well use something like > > > > sal_math_Double d; > > d.value = X; > > d.E > > > > instead. > > Yes, but this is awkward, and I don't see a benefit in using this form. Do > you? :-) It avoids the reinterpret_cast that you were after. And I don't consider it awkward, let alone awkward enough to not use it in the ca. three places where it is needed across the code base.
Let's abandon it then :-)