Skip to content
This repository has been archived by the owner on Jan 14, 2025. It is now read-only.

Work around flow analysis bug #59

Merged
merged 3 commits into from
Sep 14, 2020
Merged

Work around flow analysis bug #59

merged 3 commits into from
Sep 14, 2020

Conversation

stereotype441
Copy link
Member

Flow analysis currently has a bug preventing for-each loop variables
from being properly promoted in the presence of closures
(dart-lang/sdk#43136); as a result of this
bug the source_span package had two non-null assertions that ought to
have been unnecessary.

I'm preparing a fix for that bug, however if I land it as is, it will
cause the front end to emit errors saying "Operand of null-aware
operation '!' has type '_Highlight' which excludes null"; this in turn
will cause SDK bot breakages (since source_span is imported into the
SDK repo).

So, in order to land the fix, we need to first update the source_span
package to work around the bug; this change does that by allocating a
temporary variable (which is promoted correctly).

Once the fix for dart-lang/sdk#43136 lands,
I will make a follow-up change that deletes the temporary variable.

@stereotype441
Work around flow analysis bug
55d0f2f 
Flow analysis currently has a bug preventing for-each loop variables
from being properly promoted in the presence of closures
(dart-lang/sdk#43136); as a result of this
bug the source_span package had two non-null assertions that ought to
have been unnecessary.

I'm preparing a fix for that bug, however if I land it as is, it will
cause the front end to emit errors saying "Operand of null-aware
operation '!' has type '_Highlight' which excludes null"; this in turn
will cause SDK bot breakages (since source_span is imported into the
SDK repo).

So, in order to land the fix, we need to first update the source_span
package to work around the bug; this change does that by allocating a
temporary variable (which *is* promoted correctly).

Once the fix for dart-lang/sdk#43136 lands,
I will make a follow-up change that deletes the temporary variable.
jakemac53
jakemac53 approved these changes Sep 14, 2020
View reviewed changes
Copy link
Contributor

@jakemac53 jakemac53 left a comment

Choose a reason for hiding this comment

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

Does this need a release or is it just a temporary thing for the SDK?

Please update the pubspec/changelog if a release will be needed

@stereotype441
Copy link
Member Author

Does this need a release or is it just a temporary thing for the SDK?

Please update the pubspec/changelog if a release will be needed

No, I don't think a release will be needed. All I need for now is a SHA I can pull into the SDK so that I can avoid a chicken-and-egg problem.

After the flow analysis fix I'll be doing a follow-up change that removes the workaround. I'll update the pubspec/changelog when I do that.

@stereotype441 stereotype441 merged commit cc7c428 into dart-lang:master Sep 14, 2020
@stereotype441 stereotype441 deleted the b-43136-3 branch September 14, 2020 17:08
dart-bot pushed a commit to dart-lang/sdk that referenced this pull request Sep 16, 2020
@stereotype441
Roll latest source_span into DEPS.
27fc687 
This is needed in order to fix #43136.  See
dart-lang/source_span#59 for details.

Bug: #43136
Change-Id: I8f7ae919a255720127b1095f5879212f4b4ad169
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162781
Reviewed-by: Jake Macdonald <[email protected]>
jakemac53 added a commit that referenced this pull request Sep 18, 2020
@jakemac53
Revert "Work around flow analysis bug (#59)"
55abeae 
This reverts commit cc7c428.
@jakemac53 jakemac53 mentioned this pull request Sep 18, 2020
Merged
jakemac53 added a commit that referenced this pull request Sep 18, 2020
@jakemac53
Prep to release 1.8.0-nullsafety.1
17f16a8 
Reverts #59 and removes now unnecessary `!`s.

This requires a newer sdk than is available on dev, so run tests on be/raw/latest for now.

The dart version in flutter should already be compatible with this version.
nex3 pushed a commit to nex3/source_span that referenced this pull request Jul 2, 2024
@dependabot
Bump actions/checkout from 3.5.2 to 3.5.3 (dart-lang#59)
35657e2 
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.2 to 3.5.3.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/actions/checkout/releases">actions/checkout's releases</a>.</em></p>
<blockquote>
<h2>v3.5.3</h2>
<h2>What's Changed</h2>
<ul>
<li>Fix: Checkout Issue in self hosted runner due to faulty submodule check-ins by <a href="https://github.com/megamanics"><code>@�megamanics</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li>
<li>Fix typos found by codespell by <a href="https://github.com/DimitriPapadopoulos"><code>@�DimitriPapadopoulos</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li>
<li>Add support for sparse checkouts by <a href="https://github.com/dscho"><code>@�dscho</code></a> and <a href="https://github.com/dfdez"><code>@�dfdez</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li>
<li>Release v3.5.3 by <a href="https://github.com/TingluoHuang"><code>@�TingluoHuang</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1376">actions/checkout#1376</a></li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/megamanics"><code>@�megamanics</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li>
<li><a href="https://github.com/DimitriPapadopoulos"><code>@�DimitriPapadopoulos</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li>
<li><a href="https://github.com/dfdez"><code>@�dfdez</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3...v3.5.3">https://github.com/actions/checkout/compare/v3...v3.5.3</a></p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/actions/checkout/blob/main/CHANGELOG.md">actions/checkout's changelog</a>.</em></p>
<blockquote>
<h1>Changelog</h1>
<h2>v3.5.3</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1196">Fix: Checkout fail in self-hosted runners when faulty submodule are checked-in</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1287">Fix typos found by codespell</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1369">Add support for sparse checkouts</a></li>
</ul>
<h2>v3.5.2</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1289">Fix api endpoint for GHES</a></li>
</ul>
<h2>v3.5.1</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1246">Fix slow checkout on Windows</a></li>
</ul>
<h2>v3.5.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1237">Add new public key for known_hosts</a></li>
</ul>
<h2>v3.4.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1209">Upgrade codeql actions to v2</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1210">Upgrade dependencies</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1225">Upgrade <code>@�actions/io</code></a></li>
</ul>
<h2>v3.3.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/1045">Implement branch list using callbacks from exec function</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1050">Add in explicit reference to private checkout options</a></li>
<li>[Fix comment typos (that got added in <a href="https://redirect.github.com/actions/checkout/issues/770">#770</a>)](<a href="https://redirect.github.com/actions/checkout/pull/1057">actions/checkout#1057</a>)</li>
</ul>
<h2>v3.2.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/942">Add GitHub Action to perform release</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/967">Fix status badge</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1002">Replace datadog/squid with ubuntu/squid Docker image</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/964">Wrap pipeline commands for submoduleForeach in quotes</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1029">Update <code>@�actions/io</code> to 1.1.2</a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/1039">Upgrading version to 3.2.0</a></li>
</ul>
<h2>v3.1.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/939">Use <code>@�actions/core</code> <code>saveState</code> and <code>getState</code></a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/922">Add <code>github-server-url</code> input</a></li>
</ul>
<h2>v3.0.2</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/770">Add input <code>set-safe-directory</code></a></li>
</ul>
<h2>v3.0.1</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/762">Fixed an issue where checkout failed to run in container jobs due to the new git setting <code>safe.directory</code></a></li>
<li><a href="https://redirect.github.com/actions/checkout/pull/744">Bumped various npm package versions</a></li>
</ul>
<h2>v3.0.0</h2>
<ul>
<li><a href="https://redirect.github.com/actions/checkout/pull/689">Update to node 16</a></li>
</ul>
<h2>v2.3.1</h2>
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/actions/checkout/commit/c85c95e3d7251135ab7dc9ce3241c5835cc595a9"><code>c85c95e</code></a> Release v3.5.3 (<a href="https://redirect.github.com/actions/checkout/issues/1376">#1376</a>)</li>
<li><a href="https://github.com/actions/checkout/commit/d106d4669b3bfcb17f11f83f98e1cab478e9f635"><code>d106d46</code></a> Add support for sparse checkouts (<a href="https://redirect.github.com/actions/checkout/issues/1369">#1369</a>)</li>
<li><a href="https://github.com/actions/checkout/commit/f095bcc56b7c2baf48f3ac70d6d6782f4f553222"><code>f095bcc</code></a> Fix typos found by codespell (<a href="https://redirect.github.com/actions/checkout/issues/1287">#1287</a>)</li>
<li><a href="https://github.com/actions/checkout/commit/47fbe2df0ad0e27efb67a70beac3555f192b062f"><code>47fbe2d</code></a> Fix: Checkout fail in self-hosted runners when faulty submodule are checked-i...</li>
<li>See full diff in <a href="https://github.com/actions/checkout/compare/8e5e7e5ab8b370d6c329ec480221332ada57f0ab...c85c95e3d7251135ab7dc9ce3241c5835cc595a9">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/checkout&package-manager=github_actions&previous-version=3.5.2&new-version=3.5.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>
mosuem pushed a commit to dart-lang/tools that referenced this pull request Dec 10, 2024
@stereotype441
Work around flow analysis bug (dart-lang/source_span#59)
1637967 
Flow analysis currently has a bug preventing for-each loop variables
from being properly promoted in the presence of closures
(dart-lang/sdk#43136); as a result of this
bug the source_span package had two non-null assertions that ought to
have been unnecessary.

I'm preparing a fix for that bug, however if I land it as is, it will
cause the front end to emit errors saying "Operand of null-aware
operation '!' has type '_Highlight' which excludes null"; this in turn
will cause SDK bot breakages (since source_span is imported into the
SDK repo).

So, in order to land the fix, we need to first update the source_span
package to work around the bug; this change does that by allocating a
temporary variable (which *is* promoted correctly).

Once the fix for dart-lang/sdk#43136 lands,
I will make a follow-up change that deletes the temporary variable.
mosuem pushed a commit to dart-lang/tools that referenced this pull request Dec 10, 2024
@jakemac53
Prep to release 1.8.0-nullsafety.1
d851af1 
Reverts dart-lang/source_span#59 and removes now unnecessary `!`s.

This requires a newer sdk than is available on dev, so run tests on be/raw/latest for now.

The dart version in flutter should already be compatible with this version.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jakemac53 jakemac53 jakemac53 approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stereotype441 @jakemac53 @googlebot