https://bugs.documentfoundation.org/show_bug.cgi?id=161837
Bug ID: 161837
Summary: Drop Fraction class from tools
Product: LibreOffice
Version: unspecified
Hardware: All
OS: All
Status: UNCONFIRMED
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Severity: normal
Priority: medium
Component: LibreOffice
Assignee: libreoffice-bugs@lists.freedesktop.org
Reporter: mikekagan...@hotmail.com
In the deprecated tools library, there is a Fraction class (<tools/fract.hxx>),
that is used rather extensively, e.g. in VCL.
It has problems, including performance: e.g. it is built using boost/rational,
but to avoid dependency of each library using the class on boost, it's used in
the tools' CXX, so no inlining. But the most problematic is its unreliable
nature. It has a 'mbValid' member, which may be set to false on an intermediate
overflow; and then, the invalid fraction breaks all calculations.
Consider a valid fraction with numerator 72863296 and denominator 92331675,
approximately equal to 0.7891. Trying to multiply number 193 by this fraction
will give 0:
Fraction f(72'863'296, 92'331'675);
assert(f.IsValid());
auto x = (Fraction(193) * f).operator sal_Int32();
Here, x is suppose to be 152, but will be 0, because multiplying 193 by
72'863'296 gives 14'062'616'128, which overflows the underlying sal_Int32,
setting the multiplication result to invalid (see Fraction::operator *= in
tools/source/generic/fract.cxx).
Of course, it is possible to fix such things one by one, by introducing more
checks, branching the code to handle different cases, and hoping that now it
will give no surprises, until a next case is found. But these days, floating
point calculations are not much slower than integer; and given the lack of
inlining, rather complex logic and fragility, the use of Fraction seems to slow
down, introduce errors, and give no upsides at all.
This task is to replace all uses of Fraction in the codebase by doubles; and in
the end, drop the Fraction code from tools. This is a task that implies many
developers, so should not be assigned to yourself.
--
You are receiving this mail because:
You are the assignee for the bug.
