Editorial: Remove direct inverse asserts

[shvaikalesh]

No description provided.

[jmdyck]

The phrase _targetDesc_ is not *undefined* is, in effect, an inline assertion. Similar for the other two in this commit.

There are about 50 of these in the spec.

[ljharb]

that's true, but what's the point of the assertion here? if (x) { } else { assert(!x); } doesn't seem particularly useful :-/

[shvaikalesh]

If we agree that such assertions are extra, I am happy to remove them all in this PR.

[jmdyck]

if (x) { } else { assert(!x); } doesn't seem particularly useful :-/

My guess is, that depends on how much is between the first pair of braces. And probably varies from person to person.

Also, I think there are a few cases where the else-assertion isn't simply the logical negation of the if-condition (where you might find the assertion more useful).

[ljharb]

I can’t conceive of that (in a single if/else; i can imagine the usefulness in an if/else if chain) do you have an example?

[jmdyck]

I'm fine with replacing (some) else-inline-assertions with separate Assert steps. I just object to simply deleting all of them.

[zenparsing]

Agreed.

[ljharb]

In that case, perhaps this PR should be updated to delete only the inline else assertions in if/else chains that are a direct inverse of the if, and we can leave the rest for another PR?

[shvaikalesh]

Apologies, I was not clear enough.

I was addressing exclusively conditions with inline assertions like:

1. If X, then
  ...
2. Else !X,
  ... 

Where X is simple assertion like _fromPresent_ is true.

For me personally, they add a bit of mental overhead to make sure that X in If step is the same as X in Else.

In examples that @jmdyck kindly provided, most of the assertions are useful, but some are not:

Number.prototype.toPrecision: 10, 13
Array.prototype.copyWithin: 12.e
Array.prototype.shift: 6.e
Array.prototype.splice: 15.b.v, 16.b.v
Array.prototype.unshift: 4.c.v

[jmdyck]

I agree that In Number.prototype.toPrecision, steps 10 and 13 aren't examples of the situation I described. I wasn't talking about those, I was looking at step 10.c.iv.

All the others are of the form:

1. If _X_ is *true*, then ...
1. Else _X_ is *false*, ...

While it's debatable whether, in these cases, the else-assertion is "useful", it is an example of the situation I described, where "the else-assertion isn't simply the logical negation of the if-condition". In the logical negation of _X_ is *true*, you only know that _X_ isn't *true* -- there's a vast universe of values that it could be, of which *false* is only one. (Of course, in these cases, there's an additional constraint that _X_ is a Boolean value, which causes the else-assertion to be correct.)

[shvaikalesh]

I've came up with a simple script that detects direct inverse asserts in Else clauses (simple cases only) and reduced this PR to removing only them.

code I've used

$$("emu-alg ol").flatMap(ol => {
  const pairwise = (arr, fn) => {
    arr.reduce((prev, next) => {
      fn(prev, next)
      return next
    })
  }

  let matched = []
  let steps = [...ol.children].filter(c => c.matches("li"))

  pairwise(steps, (prev, next) => {
    let [prevText = {}, prevVar = {}] = prev.childNodes
    if (prevText.textContent != "If " || prevVar.localName != "var") return

    let [nextText = {}, nextVar = {}] = next.childNodes
    if (nextText.textContent != "Else " || nextVar.localName != "var") return

    if (prevVar.textContent == nextVar.textContent) {
      matched.push({
        if: prev.textContent.split(",")[0],
        else: next.textContent.split(",")[0],
        alg: prev.closest("emu-alg"),
      })
    }
  })

  return matched
})

[ljharb]

LGTM overall