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

Editorial: Correct usage of 'Let'/'Set' in algorithms #3131

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Editorial: Correct usage of 'Let'/'Set' in algorithms #3131

merged 1 commit into from
Aug 4, 2023

Conversation

kimjg1119
Copy link
Contributor

@kimjg1119 kimjg1119 commented Jul 26, 2023

There are some algorithms where same variable is defined twice:

Both algorithms define a (target) variable at the beginning, thus the declarations afterward are redeclaration which should be fixed.

Please note that this is a continuation of PR #2365.

HasCallInTailPosition, CaseBlock
1. Let _has_ be *false*.
2. If the first |CaseClauses| is present, let _has_ be HasCallInTailPosition of the first |CaseClauses| with argument _call_.
3. If _has_ is *true*, return *true*.
4. Let _has_ be HasCallInTailPosition of |DefaultClause| with argument _call_.
5. If _has_ is *true*, return *true*.
6. If the second |CaseClauses| is present, let _has_ be HasCallInTailPosition of the second |CaseClauses| with argument _call_.
7. Return _has_.

The variable has is defined in step 1. However, in steps 2, 4, and 6, there may be duplication since let is used to modify the value instead of set.

Evaluation, YieldExpression
1. Let _generatorKind_ be GetGeneratorKind().
2. Let _exprRef_ be ? Evaluation of |AssignmentExpression|.
3. Let _value_ be ? GetValue(_exprRef_).
...
7. Repeat,
  a. If _received_.[[Type]] is ~normal~, then
    ...
  b. Else if _received_.[[Type]] is ~throw~, then
    ...
  c. Else,
    i. Assert: _received_.[[Type]] is ~return~. 
    ii. Let _return_ be ? GetMethod(_iterator_, *"return"*).
    iii. If _return_ is *undefined*, then
      1. Let _value_ be _received_.[[Value]].
      2. If _generatorKind_ is ~async~, then
        a. Set _value_ to ? Await(_value_).
      3. Return Completion Record { [[Type]]: ~return~, [[Value]]: _value_, [[Target]]: ~empty~ }.
    ...
    viii. If _done_ is *true*, then
      1. Let _value_ be ? IteratorValue(_innerReturnResult_).
      2. Return Completion Record { [[Type]]: ~return~, [[Value]]: _value_, [[Target]]: ~empty~ }.
    ix. If _generatorKind_ is ~async~, set _received_ to Completion(AsyncGeneratorYield(? IteratorValue(_innerReturnResult_))).
    x. Else, set _received_ to Completion(GeneratorYield(_innerReturnResult_)).

The variable value is defined in step 3. However, in step 7.c.iii.1, value may be redefined since the spec uses Let to change the value, rather than Set. This potential redefinition may also occur in step 7.c.viii.1.

@ljharb
Copy link
Member

ljharb commented Jul 26, 2023

@kimjg1119 would you please register as a contributor?

@kimjg1119
Copy link
Contributor Author

@kimjg1119 would you please register as a contributor?

Submitted. Thank you for letting me know!

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Aug 3, 2023
@ljharb ljharb merged commit c316ec7 into tc39:main Aug 4, 2023
6 checks passed
@kimjg1119 kimjg1119 deleted the duplicated-assign branch August 8, 2023 04:58
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants