[CP] [dart2js] Bailout tracing of record fields when record is bailed out

[fishythefish]

Commit(s) to merge

b336f39

Target

beta and stable

Prepared changelist for beta/stable

beta: https://dart-review.googlesource.com/c/sdk/+/315240
stable: https://dart-review.googlesource.com/c/sdk/+/315242

Issue Description

Some additional context in #52968

This issue can occur in any compilation using dart2js. Our global inference phase has a tracing pass, which allows us to infer more precise types for composite values (e.g. lists, sets, maps, records) when their elements do not escape. If a value escapes, we bail out and fall back to a conservative inferred type. In particular, if a list/set/map itself escapes, we consider the values it contains to escape as well.

Due to a bug in record tracing, if a record escaped, we would stop attempting to trace its fields, but would fail to default them to conservative types. This meant these record fields would end up with narrower (possibly empty) inferred types than they should have, leading to bad codegen.

What is the fix

The fix is simply to check if we bailed out on tracing the record itself, in which case we bail out on tracing the contained value. This matches what we already do for other composite values, like lists, sets, and maps.

Why cherry-pick

The fix is small and addresses the issue, and it makes the record tracing code match what the list/set/map tracing code already does.

Risk

low

Issue link(s)

#52968

Extra Info

I've created a CP to beta. However, although the cutoff for 3.1 already happened, the release won't be for about a month. This is long enough that it might be worth considering stable as well.

/cc @sigmundch

[sigmundch]

I strongly support the cherry-pick to beta: the old behavior was unsound and customers have run into it. Risk is very low (2 line change if you exclude indentation and braces)

I'm also support the cherry-pick to stable, but I don't know what is our threshold for that at this point. @itsjustkevin should we discuss that here? or in the scrum chat?

[fishythefish]

Update: This also appears to fix #52933.

[sigmundch]

That makes me even more inclined to push for cherry-picking to stable as well.

[fishythefish]

Pre-emptive stable CP CL as well: https://dart-review.googlesource.com/c/sdk/+/315242

[itsjustkevin]

@sigmundch and @fishythefish both sound good to me