You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openoffice.apache.org by #PATHANGI JANARDHANAN JATINSHRAVAN# <JA...@e.ntu.edu.sg> on 2015/11/27 11:00:15 UTC

Patch to add bit shift functions to calc

Hi,
Can someone review this issue, it's not a very complex patch, should be a quick merge

https://bz.apache.org/ooo/show_bug.cgi?id=126701

Re: Patch to add bit shift functions to calc

Posted by Regina Henschel <rb...@t-online.de>.
Hi,

#PATHANGI JANARDHANAN JATINSHRAVAN# schrieb:
> Hi,
> Can someone review this issue, it's not a very complex patch, should be a quick merge
>
> https://bz.apache.org/ooo/show_bug.cgi?id=126701
>

I have some concerns, but I'm no professional developer and someone else 
should comment:

Why introduce a member method ScBitShiftOps together with the methods 
ScBitLShift and ScBitRShift ? For me ScBitShiftOps looks like a local 
helper function.

ScNewFunc is used as identifier for those functions, which correspond 
directly to a ocNewFunc, ScBitShiftOps does not fit into this schema.

Validity test is done, if possible, directly in ScNewFunc, especially 
the parameter count is done there.

Why restrict input to 2^48? Type "double" comes with 52 bit accuracy, 
why not use it? The specification ODF says "at least 48 bits", only the 
behavior for more bits is implementation-defined. It is inconsistent to 
forbid input values above 2^48 but accept output values above 2^48.

What is
    nFuncFmtType = NUMBERFORMAT_NUMBER;
in ScInterpreter::ScBitShiftOps in sc/source/core/tool/interpr1.cxx ?

I have not tested it yet, but I guess
   (double) result
will produce a warning. See "unsigned_int64_to_double" in 
stoc/source/typeconv/convert.cxx. Do you have compiled it on Windows?




Kind regards
Regina



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org