pawn-lang / compiler

Pawn compiler for SA-MP with bug fixes and new features - runs on Windows, Linux, macOS
Other
303 stars 70 forks source link

Switch expressions #640

Open Daniel-Cortez opened 3 years ago

Daniel-Cortez commented 3 years ago

What this PR does / why we need it:

This PR implements switch expressions, as proposed in #496. Examples of use:

GetDiscount(numitems)
{
    assert(numitems > 0);
    return switch (numitems;
        1, 2, 3: 0;  // 1-3 items - full price
        4..10:   10; // 4-10 items - 10% off
        _:       15; // >10 items - 15% off
    );
}
GetWeaponSlot(weaponid)
{
    return switch (weaponid;
        0, WEAPON_BRASSKNUCKLE:
            0;
        WEAPON_GOLFCLUB, WEAPON_NITESTICK, WEAPON_KNIFE, WEAPON_BAT,
        WEAPON_SHOVEL, WEAPON_POOLSTICK, WEAPON_KATANA, WEAPON_CHAINSAW:
            1;
        WEAPON_COLT45, WEAPON_SILENCED, WEAPON_DEAGLE:
            2;
        WEAPON_SHOTGUN, WEAPON_SAWEDOFF, WEAPON_SHOTGSPA:
            3;
        WEAPON_UZI, WEAPON_MP5, WEAPON_TEC9:
            4;
        WEAPON_AK47, WEAPON_M4:
            5;
        WEAPON_RIFLE, WEAPON_SNIPER:
            6;
        WEAPON_ROCKETLAUNCHER, WEAPON_HEATSEEKER, WEAPON_FLAMETHROWER, WEAPON_MINIGUN:
            7;
        WEAPON_GRENADE, WEAPON_TEARGAS, WEAPON_MOLTOV, WEAPON_SATCHEL:
            8;
        WEAPON_SPRAYCAN, WEAPON_FIREEXTINGUISHER, WEAPON_CAMERA:
            9;
        WEAPON_DILDO, WEAPON_DILDO2, WEAPON_VIBRATOR, WEAPON_VIBRATOR2,
        WEAPON_FLOWER, WEAPON_CANE:
            10;
        WEAPON_NIGHT_VISION, WEAPON_THERMAL_VISION, WEAPON_PARACHUTE:
            11;
        WEAPON_BOMB:
            12;
        _:  -1;
    );
}

Which issue(s) this PR fixes:

Fixes #496

What kind of pull this is:

Additional Documentation:

Y-Less commented 3 years ago

FINALLY got around to doing some merging. I'm 90% happy with this, but still not sure about the default syntax. I really think it should do away with both the _: and the ; always. So your example becomes:

GetDiscount(numitems)
{
    assert(numitems > 0);
    return switch (numitems;
        1, 2, 3: 0;  // 1-3 items - full price
        4..10:   10; // 4-10 items - 10% off
        15 // >10 items - 15% off
    );
}

That to me more nicely mirrors the for syntax, and makes it more explicit that a default is required, as it must surely always be here.

Daniel-Cortez commented 3 years ago

Forgot to mention, the last ; (the one after the default) is optional. I'm not sure if removing the _: part completely would be a good idea, but I guess I can make it optional as well.

Y-Less commented 3 years ago

I'm good with both optional.

Y-Less commented 3 years ago

Forgot to mention, the last ; (the one after the default) is optional. I'm not sure if removing the _: part completely would be a good idea, but I guess I can make it optional as well.

Actually, there is an issue with that:

new Tag:a;

new b = switch(thing;
    5: 6;
    _:a
);

Now becomes ambiguous - is that an explicit default, or an implicit default with a tag override on the value? I know I asked about tagged values in the cases, but what about in the values?

Daniel-Cortez commented 3 years ago

It's an explicit default. If you want an implicit one with a tag override, you can enclose the expression into parentheses.

return switch (value;
    1: 2;
       (_:TaggedResult())
);
Y-Less commented 3 years ago

I'm not sure if the optional _: is in the latest version of the PR, but this code gives a staging buffer overflow:

#include <a_samp>

Five()
{
    printf("5");
}

Six()
{
    printf("6");
}

main()
{
    new a = 5;
    new b = switch (a;
        5: Five();
        6: Six();
        7
    );
    printf("%d", b);
}

Also, when I change it to an explicit default:

#include <a_samp>

Five()
{
    printf("5");
}

Six()
{
    printf("6");
}

main()
{
    new a = 5;
    new b = switch (a;
        5: Five();
        6: Six();
        _: 7;
    );
    printf("%d", b);
}

I get no warnings, which is not good, because there should be warnings that Five and Six should return values.

Y-Less commented 3 years ago

I tried without a default at all:

#include <a_samp>

Five()
{
    printf("5");
    return 2;
}

Six()
{
    printf("6");
    return 1;
}

main()
{
    new a = 7;
    new b = switch (a;
        5: Five();
        6: Six();
    );
    printf("%d", b);
}

Firstly, that code took a weirdly long time to compile, it should be almost instant, but I was waiting a good few seconds. Then there's no error for missing defaults. If this is by design the fallbacks need to be documented, but I don't think they should be optional in an expression. This code actually printed 7, which I wasn't expecting.

Daniel-Cortez commented 3 years ago

I'm not sure if the optional _: is in the latest version of the PR, but this code gives a staging buffer overflow

Thanks. I'm already aware of this problem and fixed it locally yesterday, just didn't finish the tests, which is why I haven't uploaded the fix yet.

I tried without a default at all

Firstly, that code took a weirdly long time to compile, it should be almost instant, but I was waiting a good few seconds. Then there's no error for missing defaults. If this is by design the fallbacks need to be documented, but I don't think they should be optional in an expression.

No, this is not by design. That's strange; even though I wrote most of the code a year and a half ago, I clearly remember making a check for a default branch. Maybe I implemented it in a separate experimental branch and didn't merge it into the main feature branch or something...

EDIT: Indeed, it was in a separate branch; not sure why I didn't merge it in - probably because it adds a new error code for situations when a switch expression doesn't contain a default case. Anyway, I'll upload all the changes in a few hours, when I finish with the tests.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity.