Skip to content

[Compiler] Fix condition compiling #3877

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

Merged
merged 3 commits into from
Apr 16, 2025
Merged

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Apr 16, 2025

Work towards #3769

Description

There were three issues:

  1. Similar to if-statements, emit-statements also could be coming from the inherited interfaces. Thus, when compiling emit-statements, the proper elaboration must be used. (✅ Fixed)
  2. If the inherited condition refers to an unqualified-type (e.g: event, emit Event()), then the global lookup fail for the concrete type when compiling this inherited condition. (✅ Fixed).
    • This is because the event is a local type definition for the interface. Concrete type doesn't have the access.\
    • The fix was to re-write the inherited emit to be type-qualified. i.e: emit Event()emit Contract.Event()
  3. Similar to events, if the inherited condition refers to a function/imported symbol that is not available to the concrete type (can happen if the interface calls A.foo() inside the condition and interface has an import to A. But the concrete type that inherits the same condition does not import A. Then A.foo cannot be found to the compiler.) (❌ Not fixed yet).

  • 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 self-assigned this Apr 16, 2025
Copy link

github-actions bot commented Apr 16, 2025

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/compiler commit e9dea7c
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 marked this pull request as ready for review April 16, 2025 14:36
@SupunS SupunS requested a review from turbolent as a code owner April 16, 2025 14:36
@SupunS SupunS added the Bugfix label Apr 16, 2025
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

Unfortunately I also don't have a better idea than type qualifying, I guess it's fine for now

@SupunS SupunS force-pushed the supun/fix-conditions branch from a5d9465 to da5f04a Compare April 16, 2025 15:48
@SupunS SupunS merged commit 78cae27 into feature/compiler Apr 16, 2025
7 of 9 checks passed
@SupunS SupunS deleted the supun/fix-conditions branch April 16, 2025 16:19
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.

2 participants