Skip to content

For loops#366

Merged
mbenke merged 14 commits into
mainfrom
sail-for
May 16, 2026
Merged

For loops#366
mbenke merged 14 commits into
mainfrom
sail-for

Conversation

@mbenke
Copy link
Copy Markdown
Collaborator

@mbenke mbenke commented May 1, 2026

This PR adds for loops of the form

        for(i=1;i<=10;i=i+1) { s = s + i;}

Declaring variables in the initialisation block is allowed

        for(let i=1;i<=10;i=i+1) { s = s + i;}

TBD: multiple statements in the init/post blocks

@mbenke mbenke requested a review from rodrigogribeiro May 1, 2026 07:16
Comment thread src/Solcore/Backend/MastEval.hs Outdated
(_, initStmt') <- evalLoopStmt loopEnv initStmt
cond' <- evalExp loopEnv cond
(_, post') <- evalLoopStmt loopEnv post
bodies' <- mapM (fmap snd . evalLoopStmt loopEnv) body
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could cause a bug when shadowing happens inside for body.
e.g.,
code:

import std.{Num,Add,Sub,Eq,Ord,Bounded,Typedef,le};

contract C {
    function main() -> word {
        let x = 100;
        let i = 0;
        let s = 0;
        for(i=0;i<=0;i=i+1) { let x = 1; s = x; }
        return s;
    }
}

hull:

object C {
  code {

    function main () -> word {
      let x : word
      x := 100
      let i : word
      i := 0
      let s : word
      s := 0
      for ({ i := 0
           }; le$word(i, 0); { i := Add_add$word(i, 1)
                             }) { let x : word
                                  x := 1
                                  s := 100
                                }
      return s
}

| AsmBlock {Asm $1}
| 'if' '(' Expr ')' Body %shift {If $3 $5 []}
| 'if' '(' Expr ')' Body 'else' Body {If $3 $5 $7}
| 'for' '(' ForClauseStmt ';' Expr ';' ForClauseStmt ')' Body {For $3 $5 $7 $9}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We probably need to disallow let in the for post clause in either the parser or the type checker.
Currently, the below code can be compiled without an error.

import std.{Num,Add,Sub,Eq,Ord,Bounded,Typedef,le};

contract C {
    function main() -> word {
        let i = 0;
        let s = 99;
        for(i=0;i<=0;let j=1) { s = j; i = i + 1; }
        return s;
    }
}

Comment thread src/Solcore/Backend/EmitHull.hs Outdated
(condExp, condStmts) <- emitExp cond
postStmts <- emitStmt post
bodyStmts <- concat <$> mapM emitStmt body
-- Hoist allocs (from init/cond/post) into an outer block so variables
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This causes a duplicated let problem in yul.
e.g.,
code:

import std.{Num,Add,Sub,Eq,Ord,Bounded,Typedef,le};

contract Prefor {
    function main() -> word {
        let i = 100;
        let s = 0;
        for(let i=1;i<=10;i=i+1) { s = s + i; }
        return s;
    }
}

yul:

    function usr$main () -> _result {
      let i
      i := 100
      let s
      s := 0
      let i
      for {i := 1 let _v5 let _v6 _v5 := usr$le$word(i, 10)} _v5 {_v6 := usr$Add_add$word(i, 1) i := _v6 _v5 := usr$le$word(i, 10)}
      {let _v7
       _v7 := usr$Add_add$word(s, i)
       s := _v7}
      _result := s
      leave
    }

@mbenke
Copy link
Copy Markdown
Collaborator Author

mbenke commented May 9, 2026

@Y-Nak thanks for the thorough review. I tried to fix the issues you found, please look again.

@mbenke mbenke requested a review from Y-Nak May 9, 2026 06:58
Copy link
Copy Markdown
Collaborator

@rodrigogribeiro rodrigogribeiro left a comment

Choose a reason for hiding this comment

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

Great work!

Copy link
Copy Markdown
Member

@Y-Nak Y-Nak left a comment

Choose a reason for hiding this comment

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

LGTM!

@mbenke mbenke marked this pull request as ready for review May 16, 2026 15:13
@mbenke mbenke merged commit 99c9031 into main May 16, 2026
4 checks passed
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.

3 participants