Skip to content

Commit

Permalink
fix: Don't output continue/break blocks if they are not required
Browse files Browse the repository at this point in the history
  • Loading branch information
tristanmenzel committed Feb 14, 2025
1 parent 988f1f3 commit ac5691a
Show file tree
Hide file tree
Showing 135 changed files with 11,208 additions and 2,725 deletions.
22 changes: 13 additions & 9 deletions src/awst_build/ast-visitors/function-visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { FunctionPType, ObjectPType } from '../ptypes'
import { getSequenceItemType } from '../ptypes/util'
import { typeRegistry } from '../type-registry'
import { BaseVisitor } from './base-visitor'
import { maybeNodes } from './util'

// noinspection JSUnusedGlobalSymbols
export class FunctionVisitor
Expand Down Expand Up @@ -220,11 +221,11 @@ export class FunctionVisitor
sourceLocation,
},
this.accept(node.statement),
ctx.continueTarget,
...maybeNodes(ctx.hasContinues, ctx.continueTarget),
incrementor,
),
}),
ctx.breakTarget,
...maybeNodes(ctx.hasBreaks, ctx.breakTarget),
]
}

Expand All @@ -250,9 +251,9 @@ export class FunctionVisitor
sourceLocation,
sequence: requireInstanceBuilder(this.accept(node.expression)).iterate(sourceLocation),
items,
loopBody: nodeFactory.block({ sourceLocation }, this.accept(node.statement), ctx.continueTarget),
loopBody: nodeFactory.block({ sourceLocation }, this.accept(node.statement), ...maybeNodes(ctx.hasContinues, ctx.continueTarget)),
}),
ctx.breakTarget,
...maybeNodes(ctx.hasBreaks, ctx.breakTarget),
)
}
visitForInStatement(node: ts.ForInStatement): awst.Statement | awst.Statement[] {
Expand Down Expand Up @@ -306,16 +307,19 @@ export class FunctionVisitor
loopBody: nodeFactory.block(
{ sourceLocation },
this.accept(node.statement),
ctx.continueTarget,
...maybeNodes(ctx.hasContinues, ctx.continueTarget),
nodeFactory.ifElse({
condition: this.evaluateCondition(node.expression, true),
sourceLocation,
ifBranch: nodeFactory.block({ sourceLocation }, nodeFactory.goto({ sourceLocation, target: ctx.breakTarget.label })),
ifBranch: nodeFactory.block(
{ sourceLocation },
nodeFactory.goto({ sourceLocation, target: this.context.switchLoopCtx.getBreakTarget(undefined, sourceLocation) }),
),
elseBranch: null,
}),
),
}),
ctx.breakTarget,
...maybeNodes(ctx.hasBreaks, ctx.breakTarget),
)
}
visitWhileStatement(node: ts.WhileStatement): awst.Statement | awst.Statement[] {
Expand All @@ -327,9 +331,9 @@ export class FunctionVisitor
nodeFactory.whileLoop({
sourceLocation,
condition: this.evaluateCondition(node.expression),
loopBody: nodeFactory.block({ sourceLocation }, this.accept(node.statement), ctx.continueTarget),
loopBody: nodeFactory.block({ sourceLocation }, this.accept(node.statement), ...maybeNodes(ctx.hasContinues, ctx.continueTarget)),
}),
ctx.breakTarget,
...maybeNodes(ctx.hasBreaks, ctx.breakTarget),
)
}
visitContinueStatement(node: ts.ContinueStatement): awst.Statement | awst.Statement[] {
Expand Down
3 changes: 3 additions & 0 deletions src/awst_build/ast-visitors/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function maybeNodes<T>(condition: boolean, ...nodes: T[]): T[] {
return condition ? nodes : []
}
4 changes: 2 additions & 2 deletions src/awst_build/context/switch-loop-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ export class SwitchLoopContext {
getBreakTarget(label: ts.Identifier | undefined, sourceLocation: SourceLocation): string {
const labelName = label?.text
const item = this.switchLoopStack.toReversed().find(({ label }) => labelName === undefined || label === labelName)
codeInvariant(item, 'Break must must exist inside a switch or loop construct', sourceLocation)
codeInvariant(item, 'Break must exist inside a switch or loop construct', sourceLocation)
item.numBreaks++
return `${item.uniqueName}${breakSuffix}`
}

getContinueTarget(label: ts.Identifier | undefined, sourceLocation: SourceLocation): string {
const labelName = label?.text
const item = this.switchLoopStack.toReversed().find(({ label }) => labelName === undefined || label === labelName)
codeInvariant(item?.type === 'loop', 'Continue must must exist inside a loop construct', sourceLocation)
codeInvariant(item?.type === 'loop', 'Continue must exist inside a loop construct', sourceLocation)
item.numContinues++
return `${item.uniqueName}${continueSuffix}`
}
Expand Down
3 changes: 2 additions & 1 deletion src/puya/log-deserializer.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import upath from 'upath'
import { z } from 'zod'
import { SourceLocation } from '../awst/source-location'
import { logger, LogLevel } from '../logger'
Expand All @@ -22,7 +23,7 @@ export function deserializeAndLog(logText: string) {

const sourceLocation = log.location
? new SourceLocation({
file: log.location.file,
file: upath.normalize(log.location.file),
line: log.location.line,
endLine: log.location.end_line ?? log.location.line + 1,
column: log.location.column,
Expand Down
40 changes: 34 additions & 6 deletions tests/approvals/do-loops.algo.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,37 @@
import type { uint64 } from '@algorandfoundation/algorand-typescript'
import { Uint64 } from '@algorandfoundation/algorand-typescript'
import { Contract, Uint64 } from '@algorandfoundation/algorand-typescript'

function test_do(stop: uint64) {
let i = Uint64(0)
do {
i += 1
} while (i < stop)
export class DoLoopsAlgo extends Contract {
testDo(stop: uint64) {
let i = Uint64(0)
do {
i += 1
} while (i < stop)
return i
}
testDoBreak(stop: uint64, breakMod: uint64) {
let total = Uint64(0)
let i = Uint64(0)
do {
if (i > 0 && i % breakMod === 0) break

i += 1
total += i
} while (i < stop)
return total
}
testDoContinue(stop: uint64, mod: uint64) {
let i = Uint64(0)
let total = Uint64(0)
do {
if (i > 0 && i % mod === 0) {
total += 2
i += 1
continue
}
total += 1
i += 1
} while (i < stop)
return total
}
}
6 changes: 3 additions & 3 deletions tests/approvals/for-loops.algo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ class ForLoopsAlgo extends Contract {
let total = Uint64(0)
outer: for (let i = start; i < stop; i += step) {
for (let j = start; j < stop; j += step) {
total += j + j
// TODO: Can't unconditionally break as it leads to unreachable blocks and puya goes 💥
if (j === start) break outer
total += i + j

if (i * j > stop) break outer
}
}
return total
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ subroutine test(): void
total: uint64 = 0
for ({ a: a, b: _ } in items) {
total: uint64 = total + a
#loop₁ᶜ:
}
#loop₁ᵇ:
assert(total == 6)
}
Original file line number Diff line number Diff line change
Expand Up @@ -817,37 +817,11 @@
],
"label": null,
"comment": null
},
{
"_type": "Block",
"source_location": {
"file": "tests/approvals/destructuring-iterators.algo.ts",
"line": 11,
"end_line": 11,
"column": 2,
"end_column": 28
},
"body": [],
"label": "#loop₁ᶜ",
"comment": null
}
],
"label": null,
"comment": null
}
},
{
"_type": "Block",
"source_location": {
"file": "tests/approvals/destructuring-iterators.algo.ts",
"line": 11,
"end_line": 11,
"column": 2,
"end_column": 28
},
"body": [],
"label": "#loop₁ᵇ",
"comment": null
}
],
"label": null,
Expand Down
Loading

0 comments on commit ac5691a

Please sign in to comment.