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

False positive warning about "returning an abrupt completion" #619

Open
Jack-Works opened this issue Sep 27, 2024 · 9 comments
Open

False positive warning about "returning an abrupt completion" #619

Jack-Works opened this issue Sep 27, 2024 · 9 comments

Comments

@Jack-Works
Copy link
Member

it warns for the following code after upgrading from 19.1.0 to 20.0.0. It uses ReturnIfAbrupt.

Warning: spec.emu: this algorithm is declared as returning an abrupt completion, but there is no step which might plausibly return an abrupt completion

<emu-clause id="sec-iterator-step-cached" type="abstract operation">
    <h1>
    IteratorStepCached (
        _iterator_: an Iterator Record,
        _cacheGroup_: a %Map%,
    ): either a normal completion containing either an ECMAScript language value or ~not-matched~, or an abrupt completion
    </h1>
    <dl class="header">
    </dl>
    <emu-alg>
    1. Assert: _cacheGroup_ is created by CreateMatchCache and used internally for pattern-matching.
    1. If _iterator_.[[Done]] is *true*, return ~not-matched~.
    1. Let _cache_ be GetMatchCache(_iterator_, _cacheGroup_).
    1. Let _iteratedValues_ be ! Get(_cache_, *"IteratedValues"*).
    1. Let _iteratorResult_ be Completion(IteratorStep(_iterator_)).
    1. If _iteratorResult_ is an abrupt completion, set _iterator_.[[Done]] to *true*.
    1. ReturnIfAbrupt(_iteratorResult_).
    1. If _iteratorResult_ is *false*, then
        1. Set _iterator_.[[Done]] to *true*.
        1. Return ~not-matched~.
    1. Let _value_ be Completion(IteratorValue(_iteratorResult_)).
    1. If _value_ is an abrupt completion, set _iterator_.[[Done]] to *true*.
    1. ReturnIfAbrupt(_value_).
    1. Perform ! Call(<emu-xref href="#sec-array.prototype.push">%Array.prototype.push%</emu-xref>, _iteratedValues_, « _value_ »).
    1. Return _value_.
    </emu-alg>
</emu-clause>
@ljharb
Copy link
Member

ljharb commented Sep 27, 2024

Should it instead say Perform ! _value _, or just prepend a ! on the Call step that references it?

@bakkot
Copy link
Contributor

bakkot commented Sep 27, 2024

_value_ gets unwrapped by ReturnIfAbrupt already; the problem is that the new logic is failing to account for ReturnIfAbrupt and friends.

That said, I'd write the above code as

1. If _value_ is an abrupt completion, then
  1. Set _iterator_.[[Done]] to *true*.
  1. Return ? _value_.

instead of using ReturnIfAbrupt.

(Though in fact the correct thing these days is to use IteratorStepValue, which handles setting _iterator_.[[Done]] already.)

@ljharb
Copy link
Member

ljharb commented Sep 27, 2024

Right, I meant, remove the ReturnIfAbrupt step and let a ! handle it in the next step.

@bakkot
Copy link
Contributor

bakkot commented Sep 27, 2024

You can't use ! because it might actually be abrupt, at least as currently written. But yes, you can unwrap with Set _iteratorResult_ to ? _iteratorResult_., and that would also be better.

@bakkot
Copy link
Contributor

bakkot commented Sep 27, 2024

(I actually kind of want to get rid of ReturnIfAbrupt entirely, though we'll still need IfAbruptCloseIterator and IfAbruptRejectPromise.)

@Jack-Works
Copy link
Member Author

Can I 1. ? _value_?

@bakkot
Copy link
Contributor

bakkot commented Sep 29, 2024

No, but you can Set _value_ to ? _value_. Or if you already know that _value_ is definitely an abrupt completion, you can Return ? _value_.

@ljharb
Copy link
Member

ljharb commented Sep 29, 2024

You can also Perform ? _value_. but the assignment is probably clearer.

@bakkot
Copy link
Contributor

bakkot commented Sep 29, 2024

We only use Perform when invoking AOs or SDOs.

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

No branches or pull requests

3 participants