JdeRobot / PyOnArduino

4 stars 2 forks source link

Incorrect Parentheses #17

Closed rinz13r closed 5 years ago

rinz13r commented 5 years ago

The following snippet

def f():
    if (true):
        return 1

produces

void f( ) {
    if (true return ;
}
}

Also the following snippet

def f():
    if (1+1 == 2):
        a = 1
        return 1

produces

void f() {
if (((1 + 1) == 2)DynType a;a.tvar = INT;String har0 = "1";har0.toCharArray(a.data, MinTypeSz);
  return ;
}
}

Looks like string ") {" (after the condition in the if statement) is not appended.

sergiopaniego commented 5 years ago

You're right, that functionality is not yet implemented and it could be a good idea as an improvement! The problem doesn't happen when the condition is a function call but it does otherwise.

     if halduino.isButtonPressed():
        halduino.playBuzzer(330, 1000)
    elif halduino.isButtonReleased() is True:
        halduino.playBuzzer(249, 1000)

Outputs:

if (isButtonPressed()) {
    DynType var0;var0.tvar = INT;String har0 = "330";har0.toCharArray(var0.data, MinTypeSz);
    DynType var1;var1.tvar = INT;String har1 = "1000";har1.toCharArray(var1.data, MinTypeSz);
    playBuzzer(var0,var1);
 } else if ((isButtonReleased() == true)) {
    DynType var2;var2.tvar = INT;String har2 = "249";har2.toCharArray(var2.data, MinTypeSz);
    DynType var3;var3.tvar = INT;String har3 = "1000";har3.toCharArray(var3.data, MinTypeSz);
    playBuzzer(var2,var3);
 }
rinz13r commented 5 years ago

My understanding is that after an if statement is encountered, the job of balancing the parentheses is up to other visitors. vars.is_if is set to true, once an if statement is encountered and other visitors append the string ") {" if (vars.is_if == True). Now, if I'm not wrong, that condition is not being checked at many places (such as visit_Expr, visitCompare, etc.) leading to such an error. One solution is to update the implementation to make specifics such as ") {" localized in the appropriate visitors. In this case, we append "(" before and ") {"_ after parsing the predicate. Then we make a call to generic_visit for visiting the condition. However, this would break the existing code at multiple places. But this approach would allow for future constructs to be added without much hassle.

The other solution is to add ") {" at appropriate places by checking if (vars.is_if == True). However, this approach is error-prone and would lead to multiple code redundancies.

I would like to know what you think.

sergiopaniego commented 5 years ago

I see your point and I think your solution could be interesting to be implemented in order to get rid of this error now and future ones. You can code the approach if you want to and we can discuss it having the code 😄

sergiopaniego commented 5 years ago

Merged pull request and problem solved 😄