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

[Compiler] Support compilation for conditional returns #3816

Open
wants to merge 6 commits into
base: feature/compiler
Choose a base branch
from

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Mar 20, 2025

Closes #3772

Description

Design:

When a return statement is being compiled, if the enclosing function:

  • Doesn't have post conditions, then return in-place (same as before).
  • Has post conditions:
    • Instead of emitting a return, jump to the start of the post conditions (post conditions are at the bottom of the function, after all the regular statements)
    • At the end of post conditions, emit the return.

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@SupunS SupunS added the Feature label Mar 20, 2025
@SupunS SupunS self-assigned this Mar 20, 2025
@SupunS SupunS changed the title Support compilation for conditional returns [Compiler] Support compilation for conditional returns Mar 20, 2025
@SupunS SupunS force-pushed the supun/compile-returns branch 2 times, most recently from 5dcf14f to af6cffb Compare March 20, 2025 18:38
@SupunS SupunS force-pushed the supun/compile-returns branch from af6cffb to a4fb860 Compare March 20, 2025 18:40
Copy link

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/compiler commit e6f27ac
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

@SupunS SupunS force-pushed the supun/compile-returns branch from b42d50a to 9926433 Compare March 20, 2025 18:59
@SupunS SupunS force-pushed the supun/compile-returns branch from 9926433 to 3cd94a0 Compare March 20, 2025 19:00
Comment on lines +538 to +547
desugaredDecl := d.desugarDeclaration(declaration.FunctionDeclaration).(*ast.FunctionDeclaration)
if desugaredDecl == declaration.FunctionDeclaration {
return declaration
}

return ast.NewSpecialFunctionDeclaration(
d.memoryGauge,
declaration.Kind,
desugaredDecl,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated fix: Initializers wasn't getting desugared (hence conditions in initializers wasn't working)

Comment on lines +559 to +567
prevInheritedFuncsWithConditions := d.inheritedFuncsWithConditions
prevEnclosingInterfaceType := d.enclosingInterfaceType

d.inheritedFuncsWithConditions = d.inheritedFunctionsWithConditions(compositeType)
d.enclosingInterfaceType = nil

defer func() {
d.inheritedFuncsWithConditions = prevInheritedFuncs
d.inheritedFuncsWithConditions = prevInheritedFuncsWithConditions
d.enclosingInterfaceType = prevEnclosingInterfaceType
Copy link
Member Author

Choose a reason for hiding this comment

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

Also an unrelated fix, but needed for the FT test. Composite types can be inside interfaces, in the form of events. And events have initializer functions, that should be desugar-ed/compiled. This path was uncovered by the desugar-ing of initializers (fix) above.

@SupunS SupunS marked this pull request as ready for review March 20, 2025 20:12
@SupunS SupunS requested a review from turbolent as a code owner March 20, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant