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

smarter logical combination that keeps truthy/falsy/nullish property #8873

Merged
merged 5 commits into from
Jul 31, 2024

Conversation

sokra
Copy link
Member

@sokra sokra commented Jul 30, 2024

Description

Allows to handle if (unknown && falsy) if (unknown || truthy) if (unknown ?? nullish) in static analysis.

Hint: review each commit on it's own, since the first commit is only changing to a named struct.

Testing Instructions

@sokra sokra requested a review from a team as a code owner July 30, 2024 06:38
Copy link

vercel bot commented Jul 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2024 10:49am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2024 10:49am
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2024 10:49am

Copy link
Contributor

github-actions bot commented Jul 30, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

@sokra sokra requested a review from kdy1 July 30, 2024 06:41
Copy link
Contributor

github-actions bot commented Jul 30, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@sokra sokra requested a review from bgw July 30, 2024 12:37
@ForsakenHarmony
Copy link
Member

ForsakenHarmony commented Jul 30, 2024

Allows to handle if (unknown && falsy) if (unknown || truthy) if (unknown ?? nullish) in static analysis.

Which cases were handled before this PR?
Just if (falsy) if (truthy) if (nullish)?

Alternatives {
total_nodes: usize,
values: Vec<JsValue>,
additional_property: Option<AdditionalProperty>,
Copy link
Member

@ForsakenHarmony ForsakenHarmony Jul 30, 2024

Choose a reason for hiding this comment

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

Maybe we could store the logical operation here instead, and evaluate on demand? 🤔

I guess that is ever so slightly less optimized

@sokra
Copy link
Member Author

sokra commented Jul 31, 2024

Which cases were handled before this PR?
Just if (falsy) if (truthy) if (nullish)?

The unknown was the problem. If was able to handle if (truthy || unknown) but not if (unknown || truthy).
The problem is that truthy || unknown => truthy, but unknown || truthy could be unknown (if it's truthy at runtime) or truthy, so the evaluator evaluates it to Alternatives(unknown, truthy). The PR adds an additional info to Alternatives that the combination has a truthy value.

Copy link
Member Author

sokra commented Jul 31, 2024

@sokra sokra merged commit 56277b0 into main Jul 31, 2024
58 of 60 checks passed
@sokra sokra deleted the sokra/logical-evaluation branch July 31, 2024 19:03
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 31, 2024
…ercel/turborepo#8873)

### Description

Allows to handle `if (unknown && falsy)` `if (unknown || truthy)` `if (unknown ?? nullish)` in static analysis.

Hint: review each commit on it's own, since the first commit is only changing to a named struct.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
* vercel/turborepo#8879 <!-- Niklas Mischkulnig -
Convert MDX errors into issues -->
* vercel/turborepo#8873 <!-- Tobias Koppers -
smarter logical combination that keeps truthy/falsy/nullish property -->
* vercel/turborepo#8890 <!-- Tobias Koppers - Skip
unreachable code when early return can be statically analysed -->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…ercel/turborepo#8873)

### Description

Allows to handle `if (unknown && falsy)` `if (unknown || truthy)` `if (unknown ?? nullish)` in static analysis.

Hint: review each commit on it's own, since the first commit is only changing to a named struct.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
* vercel/turborepo#8879 <!-- Niklas Mischkulnig -
Convert MDX errors into issues -->
* vercel/turborepo#8873 <!-- Tobias Koppers -
smarter logical combination that keeps truthy/falsy/nullish property -->
* vercel/turborepo#8890 <!-- Tobias Koppers - Skip
unreachable code when early return can be statically analysed -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants