Skip to content

Remove dead code from JsValue::Binary in is_truthy#94165

Merged
sampoder merged 1 commit into
canaryfrom
sp/turbopack-remove-dead-code-from-is_truthy
May 29, 2026
Merged

Remove dead code from JsValue::Binary in is_truthy#94165
sampoder merged 1 commit into
canaryfrom
sp/turbopack-remove-dead-code-from-is_truthy

Conversation

@sampoder
Copy link
Copy Markdown
Member

This is another thing flagged by @bgw's Claude:

  3. Dead duplicate match arm in is_truthy for StrictEqual (lines 2469–2494)

  ( … StrictEqual, Constant(a), Constant(b)) if a.is_value_type() => Some(a ==
  b),
  ( … StrictEqual, Constant(a), Constant(b)) if a.is_value_type() => { /* 
  same-type check */ }

  Identical pattern + guard. The second arm — the only one that handles
  cross-type equality conservatively — is unreachable. Looks like a half-applied
   fix.

I've removed the second arm with a double type check. Tests appear to still pass and I can't think of a case where we'd hit the second arm.

@sampoder sampoder requested review from a team, bgw and lukesandberg May 27, 2026 18:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Tests Passed

Commit: fe85094

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Stats skipped

Commit: fe85094
View workflow run

@@ -2459,25 +2459,6 @@ impl JsValue {
JsValue::Constant(a),
JsValue::Constant(b),
) if a.is_value_type() => Some(a == b),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this handle NaN correctly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, I don't think so, see: #94172

@sampoder sampoder requested a review from lukesandberg May 28, 2026 01:10
@sampoder sampoder merged commit 4182938 into canary May 29, 2026
427 of 434 checks passed
@sampoder sampoder deleted the sp/turbopack-remove-dead-code-from-is_truthy branch May 29, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants