Re: [Libreoffice] Change in addition for date handling (intentional change??)

Thanks for checking.... Will wait and see what Noel has to say...

The change broke more in that it also ignores EXP, MUL, DIV, MINUS, and 
NEG. I expect, however, that PLUS and MINUS are the most common 
operations (to add and subtract days and similar). I cannot think of a 
highly plausible reason for EXP, MUL, or NEG. Hmmm, or perhaps I can, 
but it is a stretch.

Certainly the change will increase performance for the cases when a date 
is not used / expected.

Off hand, I am of the opinion that a fix should at least be added for 
PLUS and MINUS.

On 01/01/2012 09:52 AM, Regina Henschel wrote:
> Hi Andrew,
>
> I don't know any about the content, but I have searched, when the 
> changes where made:
>
> The part
>     // #45465 Date needs with "+" a special handling: forces date type
>               if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
>                  aL.eType = SbxDATE;
> moved from after the switch inside the switch with commit
> 1421b098ae19a10acb4836fa3752188cf7c52eb2
> by Noel Power 06-Dec-2010
>
> and the "#if 0" is a WaE fix by Tor Lillqvist 06-Oct-2011
> 8529da08517b41bd9317714e3216bb6d487b24ee
>
> So I think, Noel should know.
>
> Kind regards
> Regina
>
> Andrew Douglas Pitonyak schrieb:
> > I thought I found a bug, I decided to take a crack at fixing the bug,
> > and then I became confused because the bug appears to be intentional.
> > Let me explain:
> >
> > Consider some comments related to Basic.
> >
> > In the last release of OOo, the statement "Now + 2" returns a date /
> > time that is two days later than now. In LibreOffice, the same statement
> > appears to return the double representation of the same. Now, off hand,
> > this appears fine, until one tries to do something like this (which
> > works in OOo, but fails in LO)
> >
> > DateValue(Now + 2)
> >
> > This is no big deal if I am writing new code, because I can compensate:
> > DateValue(CDate(Now + 2)), but t kills legacy code.
> >
> > I started poking through the code and I found
> > core/basic/source/sbx/sbxvalue.cxx
> >
> > In the version of the LO code that I have, around line 1312, I see the
> > following:
> >
> > case SbxPLUS:
> > aL.nDouble += aR.nDouble; break;
> > #if 0
> > // See 'break' on preceding line... this
> > // is unreachable code. Do not delete this
> > // #if 0 block unless you know for sure
> > // the 'break' above is intentional.
> >
> > // #45465 Date needs with "+" a special handling: forces date type
> > if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
> > aL.eType = SbxDATE;
> > #endif
> >
> > In OOo, the code ends at the break, but following the switch statement,
> > there is the following check:
> >
> > // #45465 Date braucht bei + eine Spezial-Behandlung
> > if( eOp == SbxPLUS && (GetType() == SbxDATE || rOp.GetType() == 
> > SbxDATE ) )
> > aL.eType = SbxDATE;
> >
> > So, it looks like someone did a code clean-up and the special handling
> > code was moved into the switch statement and then it was commented out.
> >
> > I was so proud that I figured out where this occurred, but, I am a bit
> > stymied as to what happened here. It seems that I have almost nothing to
> > fix because someone did this intentionally, or was this left as is
> > because whomever made the change did not understand why the code was
> > there....
> >
> > Sorry, just a bit confused as to what should be done. My opinion is that
> > a bug was introduced and that the fix is clear, move the break and let
> > the code run...
> >
> > case SbxPLUS:
> > aL.nDouble += aR.nDouble;
> > // #45465 Date needs with "+" a special handling: forces date type
> > if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
> > aL.eType = SbxDATE;
> > break;
> >
> > So, what am I missing?
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice

--
Andrew Pitonyak
My Macro Document: http://www.pitonyak.org/AndrewMacro.odt
Info:  http://www.pitonyak.org/oo.php