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

[Andrew Douglas Pitonyak]

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 at 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