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

[CP] Fix infinite loop in DDC compiler #52869

Closed
nshahan opened this issue Jul 6, 2023 · 7 comments
Closed

[CP] Fix infinite loop in DDC compiler #52869

nshahan opened this issue Jul 6, 2023 · 7 comments
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@nshahan
Copy link
Contributor

nshahan commented Jul 6, 2023

Commit(s) to merge

be6a560

Target

stable and beta

Prepared changelist for stable/beta

Stable: https://dart-review.googlesource.com/c/sdk/+/312709,
Beta: https://dart-review.googlesource.com/c/sdk/+/312710

Issue Description

Some web development compiles of libraries containing is or as with specific record types will hang and never complete.

A minimal reproduction:

main() {
  Object value = 'x';
  (a: 'a', b: value) as ({String a, String b});
}

What is the fix

Some record type comparison operations were not properly incrementing a loop variable when iterating the named elements. With this fix the execution can properly exit the loop and the compile succeeds.

Why cherry-pick

Some type operations using record types will cause the DDC to hang while compiling preventing use of either the record types or web development compiler.

A user reported hitting this in a flutter application and could not compile their app for web development.

Risk

low

Issue link(s)

#52817, flutter/flutter#129482

Extra Info

cc @sigmundch

@nshahan nshahan added the cherry-pick-review Issue that need cherry pick triage to approve label Jul 6, 2023
@sigmundch
Copy link
Member

Given the low risk and how its surfaced to users (the compiler hangs), I'm very much in favor of cherry-picking to both channels to ensure it makes it to both 3.0 and 3.1.

@itsjustkevin
Copy link
Contributor

@athomas based on the timing of the next beta release, do you have opinions on when this should be merged to beta?

@vsmenon
Copy link
Member

vsmenon commented Jul 8, 2023

lgtm

@itsjustkevin itsjustkevin added the cherry-pick-approved Label for approved cherrypick request label Jul 10, 2023
@devoncarew devoncarew added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jul 10, 2023
@itsjustkevin
Copy link
Contributor

Stable cherry-pick has landed, keeping this open until the beta cherry-pick has landed in a release.

@nshahan
Copy link
Contributor Author

nshahan commented Jul 19, 2023

@athomas Any update on landing this in the beta branch?

@sigmundch
Copy link
Member

I see a beta released after @nshahan's change landed, so it may potentially have this change. That said, the commit of the cherry-pick CL doesn't have a tag with the corresponding beta.

@athomas - do we set the tags only on commits from the main branch, or do we also do it for these ad-hoc commits that are not always a clean cherry-pick? If not, would it be possible to tag them too?

@itsjustkevin
Copy link
Contributor

@nshahan this was landed on the beta branch https://dart.googlesource.com/sdk/+log/1b6421ac0ba6e5196b58bce150495d872433151d.

@sigmundch I am going to close this to clear up the queue, please feel free to open a new issue to track the commit tag discussion 🙂.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Development

No branches or pull requests

7 participants