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

Core: Fix missing second frame in QUnit.stack() in Safari #1776

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Jul 5, 2024

Safari implements ES6 Tail-Call Optimization, which is when:

  • the file is in strict mode,
  • and, in a regular function (not async or generator),
  • and, the return statement ends in a simple function call.

Then, the current function is removed from the stack before the child function begins. TCO applies even for calls that are not recursive.

The result is that, given:

makeFakeFailure -> exampleMain -> exampleParent -> exampleCurrent ->
QUnit.stack -> sourceFromStacktrace -> new Error.

In Firefox and Chrome, e.stack is:

[0] sourceFromStacktrace (SLICED)
[1] QUnit.stack          (SLICED)
[2] exampleCurrent
[3] exampleParent
[4] exampleMain
[5] makeFakeFailure

But, in Safari, the second frame gets lost because our tiny QUnit.stack() function is a candidate for Tail-Call Optimization.

[0] sourceFromStacktrace (SLICED)
[1] exampleCurrent       (SLICED)
[2] exampleParent
[3] exampleMain
[4] makeFakeFailure

This, combined with the fact that we strip the first two frames as a way to hide internal offsets, meant that in Safari we ended up attributing failed assertions and test definitions to the parent of the caller rather than the actual caller, e.g. exampleParent() instead of exampleCurrent.

Ref https://bugs.webkit.org/show_bug.cgi?id=276187.

Safari implements ES6 Tail-Call Optimization, which is when:
- the file is in strict mode,
- and, in a regular function (not async or generator),
- and, the return statement ends in a simple function call.

Then, the current function is removed from the stack before the
child function begins. TCO applies even for calls that are not
recursive.

The result is that, given:

> makeFakeFailure -> exampleMain -> exampleParent -> exampleCurrent ->
> QUnit.stack -> sourceFromStacktrace -> new Error.

In Firefox and Chrome, `e.stack` is:

```
[0] sourceFromStacktrace (SLICED)
[1] QUnit.stack          (SLICED)
[2] exampleCurrent
[3] exampleParent
[4] exampleMain
[5] makeFakeFailure
```

But, in Safari, the second frame gets lost because our tiny
`QUnit.stack()` function is a candidate for Tail-Call Optimization.

```
[0] sourceFromStacktrace (SLICED)
[1] exampleCurrent       (SLICED)
[2] exampleParent
[3] exampleMain
[4] makeFakeFailure
```

This, combined with the fact that we strip the first two frames as
a way to hide internal offsets, meant that in Safari we ended up
attributing failed assertions and test definitions to the parent of
the caller rather than the actual caller, e.g. exampleParent() instead
of exampleCurrent.

Ref https://bugs.webkit.org/show_bug.cgi?id=276187.
@Krinkle Krinkle merged commit 52f37b8 into qunitjs:main Jul 5, 2024
10 checks passed
@Krinkle Krinkle deleted the fix-stack-safari-tco branch July 5, 2024 04:40
Krinkle added a commit that referenced this pull request Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant