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: Prefer returning static values after alias comparison #3122

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

gibson042
Copy link
Contributor

I observed in passing that the overwhelming majority of If _alias_ is *value*, … return … steps that return either _alias_ or *value* after determining that the two are equivalent opt for the latter (i.e., return *value*). It seems like a reasonable convention to be cognizant of even if not caught by linting, and this PR cleans up an outlier.

$ gawk '
  BEGIN {
    # `d` is the count of active "1. If …," scopes) and length of `state`.
    d = 0;
  }
  # Handle scope exit, printing blocks that correspond with returning
  # either the alias or the value it was compared against.
  {
    while (d > 0 &&
      (index($0, state[d, "indent"]) != 1 || $1 != "1." || index($0, state[d, "indent"] "1.") == 1)) {

      i = state[d, "start"];
      j = state[d, "end"];
      if (i <= j) print "--";
      while (i <= j) printf "%d: %s\n", i, buf[i++];
      d--;
    }
  }
  # Enter new "If _alias_ is *value*, …" scopes as appropriate.
  match($0, /([[:space:]]*)1[.] If (_[^_]*_) is ([*][^*]*[*]),/, m) {
    d++;
    state[d, "start"] = NR;
    state[d, "end"] = 0;
    state[d, "indent"] = m[1];
    state[d, "alias"] = m[2];
    state[d, "value"] = m[3];
  }
  {
    # Proceed only if inside at least one active scope.
    if (!d) next;

    # Buffer the line.
    buf[NR] = $0;

    # Update the end position of every scope for which we find
    # a corresponding "return _alias_" or "return *value*"
    # (and keep a count of which we find).
    while (match($0, /[Rr]eturn (_[^_]*_|[*][^*]*[*])[.]/, m)) {
      for (j = d; j >= 1; j--) {
        if (m[1] == state[j, "alias"]) {
          state[j, "end"] = NR;
          return_alias++;
        } else if (m[1] == state[j, "value"]) {
          state[j, "end"] = NR;
          return_value++;
        }
      }
      $0 = substr($0, RSTART + RLENGTH);
    }
  }
  END {
    printf "====\nReturn _alias_: %d\nReturn *value*: %d\n", return_alias, return_value;
  }
' spec.html
--
1852:             1. If _x_ is *NaN*, return *NaN*.
--
1883:             1. If _exponent_ is *NaN*, return *NaN*.
--
1885:             1. If _base_ is *NaN*, return *NaN*.
--
4390:           1. If _Desc_ is *undefined*, return *undefined*.
--
5926:             1. If _ny_ is *undefined*, return *undefined*.
--
5930:             1. If _nx_ is *undefined*, return *undefined*.
--
6327:         1. If _status_ is *false*, return *false*.
--
8271:         1. If _hasDuplicates_ is *true*, return *true*.
--
8277:         1. If _hasDuplicate_ is *true*, return *true*.
--
8342:         1. If _hasDuplicates_ is *true*, return *true*.
--
8369:         1. If _hasDuplicates_ is *true*, return *true*.
--
8375:         1. If _hasDuplicates_ is *true*, return *true*.
--
8399:         1. If _hasDuplicates_ is *true*, return *true*.
--
8442:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8448:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8522:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8548:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8554:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8578:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8631:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8637:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8711:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8737:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8743:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
8767:         1. If _hasUndefinedLabels_ is *true*, return *true*.
--
9120:             1. If _contained_ is *true*, return *true*.
--
9282:         1. If _inList_ is *true*, return *true*.
--
10254:             1. If _foundBinding_ is *false*, return *false*.
--
10588:             1. If _home_ is *undefined*, return *undefined*.
--
11033:             1. If _hasProperty_ is *true*, return *true*.
--
12470:           1. If _extensible_ is *false*, return *false*.
--
12644:             1. If _extensible_ is *false*, return *false*.
--
12742:           1. If _desc_ is *undefined*, then
12743:             1. Let _parent_ be ? <emu-meta effects="user-code">_O_.[[GetPrototypeOf]]</emu-meta>().
12744:             1. If _parent_ is *null*, return *undefined*.
--
12749:           1. If _getter_ is *undefined*, return *undefined*.
--
13979:             1. If _succeeded_ is *false*, return *false*.
--
14074:           1. If _succeeded_ is *false*, return *false*.
--
14077:             1. If _deleteSucceeded_ is *false*, then
14078:               1. Set _newLenDesc_.[[Value]] to ! ToUint32(_P_) + *1*<sub>𝔽</sub>.
14079:               1. If _newWritable_ is *false*, set _newLenDesc_.[[Writable]] to *false*.
14080:               1. Perform ! OrdinaryDefineOwnProperty(_A_, *"length"*, _newLenDesc_).
14081:               1. Return *false*.
--
14197:           1. If _index_ is *undefined*, return *undefined*.
--
14248:           1. If _desc_ is *undefined*, return _desc_.
--
14277:           1. If _allowed_ is *false*, return *false*.
--
14492:               1. If _value_ is *undefined*, return *undefined*.
--
15196:         1. If _booleanTrapResult_ is *false*, return *false*.
--
15198:         1. If _extensibleTarget_ is *true*, return *true*.
--
15303:           1. If _targetDesc_ is *undefined*, return *undefined*.
--
15302:         1. If _trapResultObj_ is *undefined*, then
15303:           1. If _targetDesc_ is *undefined*, return *undefined*.
15304:           1. If _targetDesc_.[[Configurable]] is *false*, throw a *TypeError* exception.
15305:           1. Let _extensibleTarget_ be ? IsExtensible(_target_).
15306:           1. If _extensibleTarget_ is *false*, throw a *TypeError* exception.
15307:           1. Return *undefined*.
--
15367:         1. If _booleanTrapResult_ is *false*, return *false*.
--
15511:         1. If _booleanTrapResult_ is *false*, return *false*.
--
15555:         1. If _booleanTrapResult_ is *false*, return *false*.
--
19985:         1. If _r_ is *undefined*, return *false*. Otherwise, return _r_.
--
19994:         1. If _r_ is *undefined*, return *false*. Otherwise, return _r_.
--
20101:         1. If _r_ is *true*, return *false*. Otherwise, return *true*.
--
20118:         1. If _r_ is *true*, return *false*. Otherwise, return *true*.
--
22930:         1. If _has_ is *true*, return *true*.
--
22936:         1. If _has_ is *true*, return *true*.
--
22942:         1. If _has_ is *true*, return *true*.
--
22952:         1. If _has_ is *true*, return *true*.
--
25241:         1. If _has_ is *true*, return *true*.
--
25283:         1. If _has_ is *true*, return *true*.
--
25331:         1. If _has_ is *true*, return *true*.
--
25333:         1. If _has_ is *true*, return *true*.
--
25340:         1. If _has_ is *true*, return *true*.
--
25481:         1. If _has_ is *true*, return *true*.
--
31557:           1. If _n_ is *NaN*, return *NaN*.
--
31571:           1. If _n_ is *NaN*, _n_ > *1*<sub>𝔽</sub>, or _n_ &lt; *-1*<sub>𝔽</sub>, return *NaN*.
--
31734:           1. If _n_ is *NaN*, return *NaN*.
--
31790:           1. If _n_ is *NaN*, return *NaN*.
--
31811:             1. If _number_ is *NaN*, return *NaN*.
--
31899:             1. If _number_ is *NaN*, return *NaN*.
--
31921:             1. If _number_ is *NaN*, return *NaN*.
--
32543:           1. If _year_ is *NaN*, return *NaN*.
--
33024:           1. If _t_ is *NaN*, return *NaN*.
--
33034:           1. If _t_ is *NaN*, return *NaN*.
--
33044:           1. If _t_ is *NaN*, return *NaN*.
--
33054:           1. If _t_ is *NaN*, return *NaN*.
--
33064:           1. If _t_ is *NaN*, return *NaN*.
--
33074:           1. If _t_ is *NaN*, return *NaN*.
--
33084:           1. If _t_ is *NaN*, return *NaN*.
--
33094:           1. If _t_ is *NaN*, return *NaN*.
--
33112:           1. If _t_ is *NaN*, return *NaN*.
--
33122:           1. If _t_ is *NaN*, return *NaN*.
--
33132:           1. If _t_ is *NaN*, return *NaN*.
--
33142:           1. If _t_ is *NaN*, return *NaN*.
--
33152:           1. If _t_ is *NaN*, return *NaN*.
--
33162:           1. If _t_ is *NaN*, return *NaN*.
--
33172:           1. If _t_ is *NaN*, return *NaN*.
--
33182:           1. If _t_ is *NaN*, return *NaN*.
--
33192:           1. If _t_ is *NaN*, return *NaN*.
--
33203:           1. If _t_ is *NaN*, return *NaN*.
--
33241:           1. If _t_ is *NaN*, return *NaN*.
--
33263:           1. If _t_ is *NaN*, return *NaN*.
--
33280:           1. If _t_ is *NaN*, return *NaN*.
--
33302:           1. If _t_ is *NaN*, return *NaN*.
--
33323:           1. If _t_ is *NaN*, return *NaN*.
--
33355:           1. If _t_ is *NaN*, return *NaN*.
--
33392:           1. If _t_ is *NaN*, return *NaN*.
--
33413:           1. If _t_ is *NaN*, return *NaN*.
--
33429:           1. If _t_ is *NaN*, return *NaN*.
--
33450:           1. If _t_ is *NaN*, return *NaN*.
--
33470:           1. If _t_ is *NaN*, return *NaN*.
--
37367:               1. If _result_ is *null*, then
37368:                 1. If _n_ = 0, return *null*.
--
38350:               1. If _testResult_ is *false*, return *false*.
--
39056:               1. If _testResult_ is *true*, return *true*.
--
39053:             1. If _kPresent_ is *true*, then
39054:               1. Let _kValue_ be ? Get(_O_, _Pk_).
39055:               1. Let _testResult_ be ToBoolean(? Call(_callbackfn_, _thisArg_, « _kValue_, 𝔽(_k_), _O_ »)).
39056:               1. If _testResult_ is *true*, return *true*.
--
40054:             1. If _testResult_ is *false*, return *false*.
--
40593:             1. If _testResult_ is *true*, return *true*.
--
46082:             1. If _resultCapability_ is *undefined*, then
46083:               1. Return *undefined*.
--
49291:           1. If _t_ is *NaN*, return *NaN*.
====
Return _alias_: 3
Return *value*: 105

@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Jul 19, 2023
@bakkot
Copy link
Contributor

bakkot commented Jul 19, 2023

I've made a note to lint this, at least in simple cases.

@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 19, 2023
@ljharb ljharb force-pushed the 2023-07-compare-and-return branch from 241e1aa to e59b54b Compare July 20, 2023 21:43
@ljharb ljharb merged commit e59b54b into tc39:main Jul 20, 2023
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
editorial change establishes editorial conventions 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.

5 participants