elikaski / BF-it

A C-like language to Brainfuck compiler, written in Python
MIT License
120 stars 11 forks source link

Added Switch statements #54

Closed NeeEoo closed 3 years ago

abhra2020-smart commented 3 years ago

Wow! I wonder how you do these... Edit: PYTHON 3.10 GO BRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR

elikaski commented 3 years ago

I started reviewing this last night, I'm adding some changes and then I'll merge :)

NeeEoo commented 3 years ago

Ok, remember that you are allowed to push commits to this pull request.

elikaski commented 3 years ago

Hey @NeeEoo, I pushed 3 commits to your branch Please review and let me know if I can merge

NeeEoo commented 3 years ago

I actually had an idea that the there should be a is_literal_token. Great to see that you also got the same idea

I will have to review your changes later when I'm on my computer.

NeeEoo commented 3 years ago

Oh that's smart with the default case, including everything after it (until break) so that it can have correct flow, i thought the only way to make default have correct flow was to make a new algorithm.

The way of having the break statement outside of the scope braces, is the best we can do for now.

I noticed that the generated code sometimes contains [-][<->[-]]. But i might add that to an update to the minify code.

The example code for loop conditions you added uses <, this could be replaced with !=.


Mostly everything looks good.

When the case doesn't contain any code it includes the code from below. This was optimized in the original commited code by using has_code.

To fix this you add an if to check if the code is empty and such. This should be placed Line 276. I've came up with this

if default_code != "" or cases[case_index][2] or cases[case_index][1] != "":

I'm pretty sure that this will work.

Should i commit the changes?

Apparently your commits didn't get placed on my branch it got placed here. But i can just merge it onto the correct branch

elikaski commented 3 years ago

Good optimization idea! I saw it and forgot about during my changes I made the necessary change to support this optimization, and pushed one more commit Please take a look and let me know if its OK :)

NeeEoo commented 3 years ago

The default_code != "" is needed.

Godbolt:

0  def  5 
1  def  5 
2  def  5 
3  3 
4 
5  5 
6 
7 
8  def  5 
9  def  5 
10  def  5 
11  def  5 
12  def  5 
13  def  5 
14  def  5

Your change:

0  def  5 
1  def  5 
2  def  5 
3  3 
4 
5  5 
6 
7 
8  5 
9  5 
10  5 
11  def  5 
12  def  5 
13  def  5 
14  def  5

With default_code != "":

0  def  5 
1  def  5 
2  def  5 
3  3 
4 
5  5 
6 
7 
8  def  5 
9  def  5 
10  def  5 
11  def  5 
12  def  5 
13  def  5 
14  def  5 
NeeEoo commented 3 years ago

Code i used:

for(int i = 0; i != 15; i++) {
    printint(i);
    print(" ");
    switch(i) {
        case 3:
            print(" 3 ");
            break;
        case 4:
        case 6:
        case 7:
            break;
        case 8:
        case 9:
        case 10:
        default:
            print(" def ");
        case 5:
            print(" 5 ");
    }
    print("\n");
}
NeeEoo commented 3 years ago

And it's better to check if case_code isn't empty than just putting it on a if

elikaski commented 3 years ago

Thanks, I missed the default_code part I pushed again The "if case_code" checks if case_code is non-empty

NeeEoo commented 3 years ago

Should i merge your changes on the NeeEoo-switch-case branch to the NeeEoo/switch-case branch (this pull request) before you merge it to master? Or should you just merge the NeeEoo-switch-case branch to master?

NeeEoo commented 3 years ago

Your commits aren't on the correct branch. You just merged the first revision

elikaski commented 3 years ago

Whoops, yea I saw that I'm not an expert with GitHub :) I'll check how to fix that

NeeEoo commented 3 years ago

Just revert the merge and i can fix it before you merge