Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All the old bugfixes and patches #1

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

RayZopf
Copy link

@RayZopf RayZopf commented Feb 18, 2016

7ae61d9
needs to be tested extensively because of

syb >= 0.1.0.0 && < 0.2.0.0, fclabels > 0.4 && < 0.5, json == 0.7.*

(does it compile?)

I'm also not sure about
master...RayZopf:master#diff-c36d07f78ea57cd86da4be9be4be3014R123


less parentheses/brackets

  • make it work on OSGrid
  • fix cast of cast

state version in script output

fixes on handling variables and functions

json support and fix for string issue

update and sync version number

Review on Reviewable

modify Render.hs so that version of used LSLForge
(executable/preprocessor) is stated in processed lsl script
 - 'Render.hs' should be removed for release/when in sync
 - LslForge.hs needs to be kept in sync
 - should be kept in sync with LSLForge.cabal version number too

taken from
RayZopf/LSLForge_patched@7053b65
addressing issues:
 - https://code.google.com/p/lslforge/issues/detail?id=9#c1
  (Compiler doesn't properly detect using variable out of scope)
 - https://code.google.com/p/lslforge/issues/detail?id=1
  (Redeclaration of built-in functions doesn't show error)
 - https://code.google.com/p/lslforge/issues/detail?id=10
  (Conflict of global/local variables in modules)

for patch blame
[pells...@gmail.com](https://code.google.com/u/101374969631348043816/)

taken from
RayZopf/LSLForge_patched@c521e21
RayZopf/LSLForge_patched@c115fcb
adressing issues
 - https://code.google.com/p/lslforge/issues/detail?id=40
  (llJson* Not implemented)
 - (redone) https://code.google.com/p/lslforge/issues/detail?id=18
  (Lslforge corrupts data during optimization)

also update LSLForge cabal
given patch was not used;
 - changed version number
 - added dependency json (== 0.7.*)

blame
[pells...@gmail.com](https://code.google.com/u/101374969631348043816/)

taken from
RayZopf/LSLForge_patched@8e8b8b1
RayZopf/LSLForge_patched@27ea23d
RayZopf/LSLForge_patched@af9d403
 - sync Render.hs, LslForge.hs and LslForge.cabal
 - (0.1.6.5)

taken from
RayZopf/LSLForge_patched@5374c00
RayZopf/LSLForge_patched@d0fed91
@RayZopf
Copy link
Author

RayZopf commented Feb 18, 2016

regarding haskell packages / compile see
https://github.com/RayZopf/LSLForge_patched
and esp. https://code.google.com/p/lslforge/issues/detail?id=40#c7

@kyrahabattoir
Copy link

There is a math bug in this version with the code optimization:

llSleep((0.06 - (llGetTime() - time)) * llGetRegionTimeDilation());
is incorrectly optimized as:
llSleep((6.0e-2 - llGetTime() - time) * llGetRegionTimeDilation());

Messing up the operation order
1 - (5 - 4) = 0
1 - 5 - 4 = -8

(Mod _ _) -> 3
(Add _ _) -> 4
(Sub _ _) -> 4

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the problem be here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have no clue about the Haskell code - but could you test
1 - 1*(5 - 4)
as I asume optimizer does not recognize multiplication when removing/opimizing mathematical terms
this commit RayZopf@d16dc06
maybe here

RayZopf@d16dc06#diff-f3af7582e55ee4f0c7b8b15a86119f84R170

and here
RayZopf@d16dc06#diff-f3af7582e55ee4f0c7b8b15a86119f84L207

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to solve cast of cast bug

((string)((integer)message) == message)) misoptimized to (string)(integer)message == message)

the patch for Render.hs was changed like this:

> lo = \ t -> isLower ex t || needsBooleanParens ex t || castCast ex t
added || castCast ex t

and added this function:

> castCast :: Expr -> Ctx Expr -> Bool
> castCast ex0 (Ctx _ ex1) =
>         case ex0 of
>             (Cast _ _) -> case ex1 of
>                                (Cast _ _) -> True
>                                _ -> False
>             _ -> False
> 

@RayZopf
Copy link
Author

RayZopf commented Feb 19, 2016

I know about one other bug:

(float-variable = number-a / number-b) get's calculated to (f-v = 0);
use f-v = a.n / b.m
not (val = 4 / 5) but (val = 4.0 / 5.0)

have not tested if the bug is in main LSLForge too, or if only this patched version exposes this miscalculation

@kyrahabattoir
Copy link

Haven't seen the problem on raysilent's branch.

@raysilent
Copy link
Owner

I'm scary to merge at this point

@raysilent
Copy link
Owner

We have to compile and release this master branch though, so everyone enjoys it while we are testing these Haskell modifications. How about that?

@RayZopf
Copy link
Author

RayZopf commented Feb 19, 2016

what do you think about cherry-picking the last commit/the changes in there?

  • add version number stamp to Render.hs (lsl output)
  • sync version number for Render.hs and LslForge.hs
  • sync and update version number in Render.hs, LslForge.hs and LslForge.cabal

RayZopf@067cb56
RayZopf@4c1a497

this could also be done manually

@RayZopf
Copy link
Author

RayZopf commented Feb 19, 2016

I agree that we need someone who can code in Haskell, as the original author of those patches is not available

@RayZopf
Copy link
Author

RayZopf commented Feb 20, 2016

script to test for both issues

//Test the modified LSLForge.exe
//


float a = 99.0;
float b = 1.0;
float c = 2.0;

float a1 = 99;
float b1 = 1;
float c1 = 2;

integer ia = 99;
integer ib = 3;
integer ic = 2;
integer id = 1;

integer ia1 = 99;
integer ib1 = 3;
integer ic1 = 2;
integer id1 = 1;

integer ia2 = 99;
integer ib2 = 3;
integer ic2 = 2;
integer id2 = 1;

integer ia3 = 99;
integer ib3 = 3;
integer ic3 = 2;
integer id3 = 1;


integer returnValue_ic()
{
    // variable not touched/changed, will be replaced/removed
    return ic;
}

integer returnValue_ic1()
{
    // this way the variable gets not optimized away
    ic1 = 2;
    return ic1;
}

integer returnValue_ic2()
{
    // this way the variable gets not optimized away

    ic2 = 2;
    return ic2;
}

integer returnValue_ic3()
{
    // variable not touched/changed, will be replaced/removed
    return ic3;
}


default
{
    state_entry()
    {
    // float variable handling
    // both should be 0.5
    // when error, bad float would be 0
    a = b / c;
    a1 = b1 / c1; //ERROR

    if (a == a1 && 0.5 == a) llSay(0, "float variable handling is fine, result =" + (string)a1 + " each");
        else llSay(0, "bad float =" + (string)a1 + " should be same as good float =" + (string)a + "; EXPECTED VALUE IS 0.5");


    // subtration and parenthesis handling
    // all should be 2
    // when error, bad sub/par would be 0

    // basic math test
    ia = ib -(ic - id); //gets completely calculated/optimized as no variable is touched/changed
    ia1 = ib1 -(ic1 - id1); //ERROR, even with variables only - variable could not get replaced by value before optimizing?
    ia2 = ib2 -1*(ic2 - id2); //test workaround
    ia3 = ib3 -1*(ic3 - id3); //test workaround and optimization/calculation

    if (ia == ia1 && ia == ia2 && ia2 == ia3 && 2 == ia) llSay(0, "basic math: sub/par handling is fine, result =" + (string)ia1 + "each");
        else llSay(0, "basic math: bad sub/par =" + (string)ia1 + " and " +(string)ia2 + " - " + (string)ia3 + ", should be same as good sub/par =" + (string)ia + "; EXPECTED VALUE IS 2");

    // full test
    ia = ia1 = ia2= 99;

    ia = ib -(returnValue_ic() - id); //gets completely calculated/optimized as no variable is touched/changed
    ia1 = ib1 -(returnValue_ic1() - id1); //ERROR
    ia2 = ib2 -1*(returnValue_ic2() - id2); //test workaround
    ia3 = ib3 -1*(returnValue_ic3() - id3); //test workaround and optimization/calculation

    if (ia == ia1 && ia == ia2 && ia2 == ia3 && 2 == ia) llSay(0, "full: sub/par handling is fine, result =" + (string)ia1 + "each");
        else llSay(0, "full: bad sub/par =" + (string)ia1 + " and " +(string)ia2 + " - " + (string)ia3 + ", should be same as good sub/par =" + (string)ia + "; EXPECTED VALUE IS 2");
    }
}
// LSL script generated - patched Render.hs (0.1.6.2): LSLForge-debug.lslp Sat Feb 20 04:48:40 Mitteleuropäische Zeit 2016
//Test the modified LSLForge.exe
//


float a = 99.0;

float a1 = 99;

integer ia = 99;

integer ia1 = 99;
integer ic1 = 2;

integer ia2 = 99;
integer ic2 = 2;

integer ia3 = 99;

integer returnValue_ic1(){
    ic1 = 2;
    return ic1;
}

integer returnValue_ic2(){
    ic2 = 2;
    return ic2;
}


default {

    state_entry() {
        a = 0.5;
        a1 = 0;
        if (a == a1 && 0.5 == a) llSay(0,"float variable handling is fine, result =" + (string)a1 + " each");
        else  llSay(0,"bad float =" + (string)a1 + " should be same as good float =" + (string)a + "; EXPECTED VALUE IS 0.5");
        ia = 2;
        ia1 = 3 - ic1 - 1;
        ia2 = 3 - 1 * (ic2 - 1);
        ia3 = 2;
        if (ia == ia1 && ia == ia2 && ia2 == ia3 && 2 == ia) llSay(0,"basic math: sub/par handling is fine, result =" + (string)ia1 + "each");
        else  llSay(0,"basic math: bad sub/par =" + (string)ia1 + " and " + (string)ia2 + " - " + (string)ia3 + ", should be same as good sub/par =" + (string)ia + "; EXPECTED VALUE IS 2");
        ia = ia1 = ia2 = 99;
        ia = 2;
        ia1 = 3 - returnValue_ic1() - 1;
        ia2 = 3 - 1 * (returnValue_ic2() - 1);
        ia3 = 2;
        if (ia == ia1 && ia == ia2 && ia2 == ia3 && 2 == ia) llSay(0,"full: sub/par handling is fine, result =" + (string)ia1 + "each");
        else  llSay(0,"full: bad sub/par =" + (string)ia1 + " and " + (string)ia2 + " - " + (string)ia3 + ", should be same as good sub/par =" + (string)ia + "; EXPECTED VALUE IS 2");
    }
}

@raysilent
Copy link
Owner

Guys, let's work on 0.1.7, attempt to prepare it for release. Then we'll get back to this issue.
Please use #2 for communications on 0.1.7.

@LeonaMorro
Copy link

Regarding the "float variable handling" ((float-variable = number-a / number-b) get's calculated to (f-v = 0);):

I don't see a bug, because the operation is done exectly like LSL behaves inworld:

float fv=1/2;
both operants are integer -> the division is an integer division
(integer division : 1/2=0)
the integer result is then magicly cast to a float, because the left side (fv) is an float and the right side is an integer.

Try the following script inworld:

default {
    state_entry() {
        float fv;
        //integer division (because both operands are integer)
        //casting to float after the division
        fv=1/5;
        llOwnerSay((string)fv); //0.000000
        //float division (because at least one operand is a float)
        fv=4.0/5;
        llOwnerSay((string)fv); //0.800000
        //float division (because at least one operand is a float)
        fv=4/5.0;
        llOwnerSay((string)fv); //0.800000
        //float division (because at least one operand is a float)
        fv=4.0/5.0;
        llOwnerSay((string)fv); //0.800000
    }
}

keep in sync with upstream
@RayZopf
Copy link
Author

RayZopf commented Feb 20, 2016

tested float/autocast again - see attached script
LSLForge-debug.txt

basically the LSLForge.exe in this PR does:
LSLForge error only - autocasts given values, disregardes type of used variable in calculation
i.e. float c = 3 used in float b = 10 / c is treated as integer value, resulting in b = 3.000000
Mono (and LSLForge):
used values are of type integer, so result becomes integer too - before putting into float variable
i.e. calculating float a2 = 10 / 3 = 3.000000

do I have to like that?

@LeonaMorro
Copy link

Mono (and LSLForge):
used values are of type integer, so result becomes integer too - before putting into float variable
i.e. calculating float a2 = 10 / 3 = 3.000000

do I have to like that?

I don't know ;) .. But I wouldn't call it an error or bug. I guess it depends on the point of view .. you could also call it a feature, because an integer division is much faster than a float division. (And because both operants are integers, the interpreter could/should guess you would like to perform an integer division)

LSLForge-debugWithComments.txt

@kyrahabattoir
Copy link

I wouldn't merge that >_>

@LeonaMorro
Copy link

float a2 = 10 / 3 (= 3.000000)
is calculated inworld

@RayZopf
Copy link
Author

RayZopf commented Feb 22, 2016

I can live with this:

//As a workaround
//Never ever trust an autocast, if you want a number to be an float, write it as a float

and given the findings, I would not call it a workaround - esp. as it is needed for any LSL script to run correctly (#1 (comment))
That way LSLForge autocasting float c = 3 to integer value 3 won't happen too.
Also I cannot imagine any other operation where LSLForge inadvertently would optimize/cast a value as integer...
BTW. LSO gives same results as Mono :D

@RayZopf
Copy link
Author

RayZopf commented Feb 22, 2016

just checked - LSLForge previous to above changes also does autocast integer values within float variables to integer

float a1 = 99;
float b1 = 1;
float c1 = 2;
a1 = b1 / c1;

= 0 (integer)

this is still different to Mono / inworld variable autocast handling- but this behaviour has not been changed by applied patch!
try with the different LSLForge revisions and confirm, please

partly reverted applied patch
- ugly
- only used on Sub, Div and Mod operations
- adding back some unnecessary parenthesis too
logic expressions handling (And, Or) seems to be untouched and still
correct
added comments
partly reverted applied patch
- ugly
- used on ShiftL and ShiftR too
- adds back some unnecessary parenthesis too

 thanks for pointing out @furroy
Handle associativity similar to precedence
- modified/moved Cast of Cast handling
- all operators should be covered now
- adds too much parenthesis in some cases
@RayZopf
Copy link
Author

RayZopf commented Feb 26, 2016

latest win32 exe files for testing,
unstripped and stripped version

also one single change that I have not commited yet
old:

    if assoc ex0 && assoc ex1 then True
        else False
--may give a few too much parenthesisas, as not comparing e0 and ex1 e.g. (Sub _ _) == (Sub _ _)
--would be no need for parenthesis when e.g (Sub _ _) and (Lt _ _) except for precedence
--but be carefull with Lt, Gt, Le, Ge

now:

    if assoc ex0 && assoc ex1 && (prec ex0 == prec ex1) then True
        else False

LSLForge.exe.zip
LSLForge.exe.stripped.zip

- also check for precedence there
- late commit, change already mentioned in PR
@RayZopf
Copy link
Author

RayZopf commented Dec 13, 2016

@PellSmit could you take a look into this pull request, please?
I believe most of the major enhancements / patches were done by you,
I only contributed text, description and a fix to that parenthesis code

@PellSmit
Copy link

PellSmit commented Feb 9, 2017

@RayZopf,I don't have enough time to do it.
Sorry, I think I'll make you want for long time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants