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

feat: return with no expression. #3034

Closed
spapinistarkware opened this issue May 4, 2023 · 12 comments · Fixed by #3134
Closed

feat: return with no expression. #3034

spapinistarkware opened this issue May 4, 2023 · 12 comments · Fixed by #3134
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed small

Comments

@spapinistarkware
Copy link
Contributor

"return;" should be equivalent to "return ():".

Add syntax to cairo_spec.rs and parser.rs.
Fix what breaks.

@spapinistarkware spapinistarkware added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed small labels May 4, 2023
@gaetbout
Copy link
Contributor

gaetbout commented May 4, 2023

I'll look into it

@milancermak
Copy link
Contributor

Can the same be done for break please?

@gaetbout
Copy link
Contributor

gaetbout commented May 4, 2023

If @spapinistarkware confirms it is the same complexity, I'll try

@spapinistarkware
Copy link
Contributor Author

I think it's actualy easier to change both :)

@milancermak
Copy link
Contributor

You're welcome @gaetbout 😁

@gaetbout
Copy link
Contributor

gaetbout commented May 5, 2023

Hey @spapinistarkware I'm struggling a bit (just doing return atm):
I'm modifying:

    .add_struct(StructBuilder::new("StatementReturn")
        .node("return_kw", "TerminalReturn")
        .node("expr", "Expr")
        .node("semicolon", "TerminalSemicolon")
    )

to

.add_struct(StructBuilder::new("StatementReturn")
    .node("return_kw", "TerminalReturn")
    **.node("optinal_expr", "OptionExpr")**
    .node("semicolon", "TerminalSemicolon")
)

then running the command
cargo run --bin generate-syntax
To update the AST

Then I go to parser.rs to update:

SyntaxKind::TerminalReturn => {
    let return_kw = self.take::<TerminalReturn>();
    let expr = self.parse_expr();
    let semicolon = self.parse_token::<TerminalSemicolon>();
    Some(StatementReturn::new_green(self.db, return_kw, expr, semicolon).into())
}

To update it to something like:

SyntaxKind::TerminalReturn => {
    let return_kw = self.take::<TerminalReturn>();
   let optional_expr = if self.peek().kind == SyntaxKind::**???** { // Should it be StatementExpr?
        self.parse_expr();
    } else {
        **???**
    };
    let semicolon = self.parse_token::<TerminalSemicolon>();
    Some(StatementReturn::new_green(self.db, return_kw, expr, semicolon).into())
}

I tried a bunch of stuff but can't figure out what to do 🙁 ...
Any help would be ... Helpful 😄

@spapinistarkware
Copy link
Contributor Author

Try to check if the next token is semicolon. If not, parse expr.
After parse_expr(), do .into()
At the other branch fo something like OptionalExprNone::new_green

@spapinistarkware
Copy link
Contributor Author

spapinistarkware commented May 6, 2023 via email

@gaetbout
Copy link
Contributor

gaetbout commented May 6, 2023

That's the thing I don't have any of those two being generated.
I must be missing something

@gaetbout
Copy link
Contributor

Status update:
I have the syntax working and compiling but I'm encountering a bug in this specific case:

fn some_fn(){
   let mut a = 12;
   loop {
      if a == 14 {
         break;
      }
      a =  a + 1;
   }

error: Variable not dropped. Trait has no implementation in context: core::traits::Drop::<core::never>. Trait has no implementation in context: core::traits::Destruct::<core::never>.
 --> bool_test.cairo:25:5
    loop {
    ^****^

Looking into it.

@yuvalsw
Copy link
Contributor

yuvalsw commented May 11, 2023

This is a lowering error. Assuming you didn't change anything in the lowering phase, this is unrelated.
If you break with a value, does it work?

@gaetbout
Copy link
Contributor

if I break with (); it does work.
Trying to look into the lowering differences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants