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

--- Comment #8 from [email protected] ---
(In reply to Pathangi Jatinshravan from comment #7)
> (In reply to damjan from comment #5)
> > (In reply to Pathangi Jatinshravan from comment #4)
> > > (In reply to damjan from comment #3)
> > > > (In reply to Pathangi Jatinshravan from comment #2)
> > > > > Created attachment 85143 [details]
> > > > > Patch with review comments incorporated
> > > > > 
> > > > > I have made it such that at only two arguments are supported, and by 
> > > > > using
> > > > > sal_uInt64, ensured that result up to 2^48 - 1 are also supported. 
> > > > > Also
> > > > > dropped unnecessary logging line.
> > > > 
> > > > It doesn't compile (FreeBSD 10.2, Clang 3.4.1):
> > > > 
> > > > /AOO/main/sc/source/core/tool/interpr1.cxx:1166:37: error: expected a 
> > > > class
> > > > or namespace
> > > >         if (bitOp == ScInterpreter::bitArithmetic::BITAND)
> > > >                      ~~~~~~~~~~~~~~~^
> > > > /AOO/main/sc/source/core/tool/interpr1.cxx:1168:42: error: expected a 
> > > > class
> > > > or namespace
> > > >         else if (bitOp == ScInterpreter::bitArithmetic::BITOR) 
> > > >                           ~~~~~~~~~~~~~~~^
> > > > /AOO/main/sc/source/core/tool/interpr1.cxx:1170:42: error: expected a 
> > > > class
> > > > or namespace
> > > >         else if (bitOp == ScInterpreter::bitArithmetic::BITXOR) 
> > > >                           ~~~~~~~~~~~~~~~^
> > > 
> > > 
> > > Can you try enabling c++11 on the compiler? The scope resolution for enum
> > > was added in c++11
> > 
> > Our ports to OS/2, Solaris, Android etc. won't necessarily have c++11 
> > either.
> 
> The latest patch fixes this issue.

Yes it does, thank you :-).

I've found the following problems though:
1. doOperation() is only used in the file it's defined in, but since it's a
global function, any other doOperation anywhere in AOO will clash with it,
which is likely since it's very general name. Please make it "static" to limit
visibility to that file, or put it in a namespace.
2. A1=1, B1=Hi, C1=BITAND(A1;B1) works and gives 1, because you didn't handle
the error case in svSingleRef where HasCellValueData is false.
3. svDoubleRef works by applying to all values in the given range, instead of
only the single value (if any) obtained through implied intersection of ODF
spec 6.3.3. Calling GetDouble() does implied intersection for you internally
(it's defined in interpr4.cxx) by calling DoubleRefToPosSingleRef(), so I think
that's what you should be using for both the svDoubleRef and svSingleRef cases
(if not everything).

I think problems 2 and 3 were caused because you seem to have copied how the
ScInterpreter::ScAnd() and other logical functions work, which are different,
because they're defined as "AND( { Logical|NumberSequenceList L }+ )" while
BITAND is defined much stricter as "BITAND( Integer X ; Integer Y )". ODFF
section "6.3.6 Conversion to Integer" describes how you first get a number from
"6.3.5 Conversion to Number", which explains how for references to > 1 cell you
have to do "implied intersection" of section 6.3.3. The ROMAN function
implemented in ScRoman in interpr2.cxx, which is also defined as taking
Integer, just calls GetDouble(), so I suspect you should also just call
GetDouble() for all cases instead of a switch on GetStackType().

-- 
You are receiving this mail because:
You are the assignee for the issue.

Reply via email to