Ecma262: Should eval?.() be direct eval?

Created on 24 Jun 2020  ·  44Comments  ·  Source: tc39/ecma262

Issue-ization of https://github.com/tc39/test262/pull/2667#issuecomment-648442908.

eval?.() is specified as indirect eval, which is evidently intentional, but speaking as a reviewer and implementer, I'm not sure how many of us understood this to be the case.

In particular, JSC / SM / V8 all implemented and shipped what we expected the spec to say—i.e., that "code with ?. behaves the same as code without it when the LHS is non-nullish"—so we have a divergent reality here.

Whichever way we resolve this, I think it's necessary to have a proper discussion about it.
I'll also create a PR for moving to direct eval, at least for demonstration's sake.

Most helpful comment

I'm not sure whether direct eval or indirect eval is the correct question.

As specified, eval() is a statically detected special form variant of _CallExpression_. The runtime semantics of the special form is roughly:

if the current value of `eval` is %eval% (ie, the built-in eval function of the CallExpression's realm)
   then do a direct eval using the CallExpression's argument
   else  use the normal CallExpression semantics (ie, just do  normal call behavior including error checks)

So it isn't choosing between a direct eval or an indirect eval, it is choosing between a direct eval and a normal call. It might be an attempt to call something that is not callable (eg, undefined) or it might be calling any other function.

So, what does a normal programmer expect when writing eval(expr). If they have not guarded it with a predicate then they should be expecting a direct eval. If they wanted an indirect eval (whose semantics is quite different from direct eval) they would have coded it using a formulation that forces an indirect eval.

What should they expect if they intentionally wrote eval?.(expr). Presumably based upon orthogonal composition of features, the exact same behavior as eval(expr) except that the else-clause of the semantics has an additional null/undefined guard.

The simplest thing from a programmer's perspective is to just threat ?. in a call as an orthogonal feature. Any special case treatment, including a syntax error for eval just adds to the already too large set of arbitrary special case rules that JavaScript programmers need to learn.

All 44 comments

My understanding of the factors to weigh:

  • Direct eval would be more consistent behavior and thus easier for users to remember (though it's true that few will ever need to know this).

  • Indirect eval could be more useful among those that do need to know: eval?.() is easier to write than (0,eval)(), whereas if it's the same as eval() then there's no reason to write it at all.

  • Direct eval is (potentially) more straightfoward to implement, since there isn't any additional binding to incur indirection here and AST-wise one can just wrap the existing eval call node with an Optional node.

  • Spec-wise, I understand that some folks would rather not ever introduce new cases of direct eval. (_I wasn't thinking of this as a "new" case so much as rewrapping an "old" case, but since the direct eval algorithm text is strictly associated with CallExpression, it turns out we would need to duplicate it for OptionalExpression..._)

eval?.() is specified as indirect eval, which is evidently intentional,

It wasn’t exactly intentional; rather, it was intentionally unintentional. When I wrote the spec, I deliberately chose to ignore what I considered to be a non-issue.

IOW, the currently specced semantics is a consequence of how direct eval was specced at the time of introduction of optional chaining, not a deliberate decision.

* Direct eval is (potentially) more straightfoward to implement, since there isn't any additional binding to incur indirection here and AST-wise one can just wrap the existing eval call node with an Optional node.

FWIW SpiderMonkey was able to quickly change from direct to indirect eval, because for us it was a one line change: https://github.com/mozilla/gecko-dev/commit/8cc62552d63213689bff6c0ee82f16a08127ac0b

I understand that other implementations may have more trouble here.

Unless someone else from the SpiderMonkey team (cc @codehag) has any concerns about the change to indirect eval, I'll probably won't revert the commit for now, so we can report to TC39 if we encounter any web-compatibility issues (, which I expect to be rather unlikely :smile:).

I am following it. Will report back if there are any issues. I also do not suspect that there will be any.

FWIW I'm happy with eval?.() not being a new direct eval case. I don't really see the motivation for adding more direct eval cases. If we leave the semantics as is, it would probably be helpful to add a note indicating that this case is treated as indirect eval, to avoid confusing future implementers and other spec readers.

@anba, @codehag: Fair enough—this discussion is definitely predicated on both ways still being web-compatible, so if we found that not to be the case, that'd be the strongest deciding factor!

@claudepache: Thanks, that's a really helpful clarification. So then it's much like the delete x?.y case, where we did what "comes for free" in the spec, even though it requires conscious effort in implementation. I do think eval?.() is a bit more concerning though, since delete x?.y still follows the aforementioned rule of thumb.

@littledan: Adding more direct eval cases is certainly not a goal in itself, though it seems that _not_ doing so might be considered a goal among certain delegates. My own goal here is really just to have the discussion now that we forgot to have during review, and I'm trying to evaluate by priority of constituencies.

To resummarize in paragraph form:

If eval?.() were something we expected or encouraged the average user to write, then I think it would need to be direct eval to avoid confusion. Since that's not case, the situation is fuzzier. Below the level of userland, either the implementation needs to add a restriction for direct eval or the spec needs to add an allowance for it, so that part is just a question of which side of the line the burden falls on.

FWIW it looks like updating V8 to make this an indirect eval would be a single-line-ish change.

JSC would be as well; I'm sorry if I made it sound like this would imply a major refactor.

I just want our decision here to be conscious and not accidental.

@rkirsling

So then it's much like the delete x?.y case, where we did what "comes for free" in the spec, even though it requires conscious effort in implementation.

No, delete x?.y has exactly one reasonable semantics. It is much more like document.all?.foo, where different people have different expectations.

Note that eval has very peculiar semantics; for instance, it is the only function object that behaves differently depending on the name of the binding it is stored in. So, I am not particularly concerned if your particular “rule of thumb” about ?. is not satisfied either.

A third option is to make eval?.(x) a Syntax Error.

@claudepache
Sorry, I was referring specifically to your comment that "I deliberately chose to ignore what I considered to be a non-issue"—I mean that we allowed delete x?.y because it was zero cost to do so spec-side, though this did require some acrobatics implementation-side; similarly, the spec for eval?.() is currently written in a way that's zero cost spec-side but does require a conscious check implementation-side.

I do see your point in analogizing with document.all though—if we say that we resolved that issue by choosing not to proliferate undesirable legacy semantics, then I suppose the same argument could apply for indirect eval.

A third option is to make eval?.(x) a Syntax Error.

wouldn't that mean that function eval() {} ; eval?.() would be a syntax error? or would we only let it error when it's %eval%?

fwiw function eval() {} is already a syntax error in strict mode. I don't think eval?.() should be a syntax error though.

FWIW, babel and TypeScript also transpile eval?.() as a direct eval.

Note that Babel's behavior was accidental (but my opinion matches our implementation: eval?.() should be direct eval for symmetry with eval()).

cc @erights

I second @littledan's comment. Indirect eval should be just fine in this case.

@claudepache says

A third option is to make eval?.(x) a Syntax Error.

I agree. I suggest the "Principle of Least Damaging Surprise" as a name for the principle we followed for the -x**y case.

There's a syntactic construct S that some would guess has meaning M1 and some would guess has meaning M2.

  • If we give it meaning M1, then programs written by those who thought it had meaning M2 might silently deviate for their authors intentions at runtime. We need to examine how damaging M1 behavior is to programs written assuming M2.
  • and vice versa of course. How damaging is M2 behavior to programs written assuming M1? Well, if the deviation is only a surprising throw, that's not too bad. If it is a different number, that's very bad.
  • If we make S a syntax error, then both audiences are surprised, raising the number surprised. However, the consequence of the surprise is "Hmmm, that didn't work. How do I write my program so it will be accepted?" This happens only during development, not at runtime on some customer's device.

In this case, I don't think it matters much because I don't think anyone will naively write this. However, even if the effect is minor, syntax error causes the least damage.

While I think the error approach is worthy of mention, I don't think I agree that it's less surprising in this case. It might have potential to be _more_ surprising here, for the reason that @ljharb was approaching above—even in strict mode, I can freely set globalThis.eval to null or another function, so eval?.() really can be nullish or, in any case, something other than %eval%.

Given that eval has this undeniable identifier-ness, I think either semantics would still be preferable to an error.

@rkirsling Whatever we do, it will be more surprising for someone. The idea of the syntax error, is not to make the behaviour less surprising, but to make it impossible to get wrong in case they have the wrong intuition. The fact that it is an early error means that there is very low chance that it will appear by accident in production.

I'm not sure whether direct eval or indirect eval is the correct question.

As specified, eval() is a statically detected special form variant of _CallExpression_. The runtime semantics of the special form is roughly:

if the current value of `eval` is %eval% (ie, the built-in eval function of the CallExpression's realm)
   then do a direct eval using the CallExpression's argument
   else  use the normal CallExpression semantics (ie, just do  normal call behavior including error checks)

So it isn't choosing between a direct eval or an indirect eval, it is choosing between a direct eval and a normal call. It might be an attempt to call something that is not callable (eg, undefined) or it might be calling any other function.

So, what does a normal programmer expect when writing eval(expr). If they have not guarded it with a predicate then they should be expecting a direct eval. If they wanted an indirect eval (whose semantics is quite different from direct eval) they would have coded it using a formulation that forces an indirect eval.

What should they expect if they intentionally wrote eval?.(expr). Presumably based upon orthogonal composition of features, the exact same behavior as eval(expr) except that the else-clause of the semantics has an additional null/undefined guard.

The simplest thing from a programmer's perspective is to just threat ?. in a call as an orthogonal feature. Any special case treatment, including a syntax error for eval just adds to the already too large set of arbitrary special case rules that JavaScript programmers need to learn.

The simplest thing from a programmer's perspective is to just threat ?. in a call as an orthogonal feature.

Well stated. @allenwb's argument here matches my thinking exactly.

So, what does a normal programmer expect when writing eval(expr). If they have not guarded it with a predicate then they should be expecting a direct eval. If they wanted an indirect eval (whose semantics is quite different from direct eval) they would have coded it using a formulation that forces an indirect eval.

I am not sure we can assume what they want for indirect or direct evals in here.

What should they expect if they intentionally wrote eval?.(expr). (...)

If I understand correctly, we are setting some sort of a default path for usages of eval calls that are not straight up eval(x). I could say that even affects globalThis.eval(x) or window.eval(x) as indirect evals would be enough reason for such a thing like eval?.(x) being indirect as well.

I don't worry too much which direction we take here, but I believe we are creating an exception if we change the spec, rather than enforcing an intended default behavior.

Today, what we have as default is: direct eval is only direct calls to a name eval that is locally available. Indirect eval is everything else, even an expression that evaluates to eval such as (0, eval) or window.eval.

If we change the specs as proposed, we add a new specific rule such as: direct eval is also possible through optional chain. We remove something out from the "everything else" bag of indirect eval.


On another note, I don't think it's ECMAScript's responsibility to guard developers against eval?.(x), I'd avoid making it a syntax error.

If we change the specs as proposed, we add a new specific rule such as: direct eval is also possible through optional chain. We remove something out from the "everything else" bag of indirect eval.

No, I don't think you understand how "direct eval" is statically recognized by the specification in 12.3.6.1. The guard that is used to recognize a direct eval is:

If Type(ref) is Reference, IsPropertyReference(ref) is false, and GetReferencedName(ref) is "eval", then

The intent of this guard is to ensure that the _MemberExpression_ expression that provides the function value is exactly the _Identifier_ eval. So, eval(x) will usually be a direct eval (it must be the correct Realm specific value) but globalThis.eval(x) window.eval(x), or foo?.eval(x) is never recognized as an direct eval.

If the evaluation abstraction operation for _MemberExpression_?.(x) uses the same guarded logic as 12.3.6.1 then eval?.(x); will work exactly as eval(x); for cases where eval binds to its normal built-in value and exactly like let $$foo=eval; $$foo?.(x)` for all other cases.

I understand how it is spec'ed and I'm glad implementations are consistent, as anything from my Test262 perspective.

My point of view here is as a web dev, out of my personal intuition, putting any spec work aside and having close to 0 experience as implementor. As a web dev, writing JS code, I'd expect eval?.(x) to be indirect because I see it as odd as eval coming in from a property reference.

As I mentioned, my intuition goes as we are proposing an exception for the general rule where direct evals were limited to _roughly_ MemberExpression Arguments (out of the _CoverCallExpressionAndAsyncArrowHead_). We are now extending the rule to, as you mentioned, MemberExpression ?. Arguments.

I'm not objecting to this change, but I don't think it's the best path.

I think it's very hard to say how developers expect eval?.() be direct or indirect eval.

On one side, I believe many developers simply explain eval?.(x) as eval != null ? eval(x) : undefined so it should be direct eval.

On the other side, some developers (especially those have much deeper comprehension of direct/indirect eval) will expect it as indirect eval because they understand direct eval as an operator (think eval(x) as typeof(x)), and if the form is not look like operator it should be treated as indirect eval (typeof?.(x) is invalid so eval?.(x) is not "operator" and must be indirect call).

Both side seems reasonable, so my first feeling is we should make it as syntax error.

But we already can write let eval = () => {}; eval(), so make eval?.() as syntax error also introduce surprise, though I guess no one write such code in real world.

If we couldn't make it a syntax error, I guess maybe we can make eval?.() throw TypeError if eval is %eval%. Though it's a little bit magical. (eval is magical anyway :-)

Lending further support for syntax error, notice that

let eval = () => {}; eval()

is already a syntax error in strict code.

@erights

let eval = () => {}; eval()

The syntax error in strict mode comes from the assignment, not the _CallExpression_. If just the assignment was done within non-strict code, the strict mode _CallExpression_ would go ahead and invoke the arrow function. I don't see how the strict mode restriction on assignment to eval is relevant to discussing the semantics of eval?.().

A more relevant consideration may be consistency with other syntactic forms that look like calls. For example, import(). Presumably import?.() is a syntax error because it isn't explicitly allowed by the grammar.

In that case tho, import isn't an identifier like eval is - import can never be nullish so that import() would fail.

Consider let eval = ... already syntax error in strict codes and modules, It seems the only potential use case of eval?.(s) is assuming someone or some env will nullify globalThis.eval. But consider the CSP, it still could throw, so I guess in practice, u'd better write canEval && eval(s) (or try eval(s) as https://github.com/isiahmeadows/proposal-try-expression) instead of eval?.(s) .

So I think make it a syntax error may be the best option.

@allenwb

The syntax error in strict mode comes from the assignment, not the CallExpression. If just the assignment was done within non-strict code,

First, I'll take this rare opportunity to say " declaration, not assignment ". In fact assignment would be allowed strict, which actually weakens the point I am trying to explain. In any case, that point is

For programmers just writing normal strict code, it would be very rare for eval to be bound to anything other than %eval%. I took an earlier comment to raise the issue of a programmer being surprised if they just innocently happen to use eval for one of their own function names. This will not arise innocently among programmers writing strict code. Therefore, the syntax error damages no one.

Programmers who bind eval sloppy and then use it strict are in it deep enough that they'll know what to do when their weird code is rejected by this syntax error.

I agree that it is doubtful whether there is any reasonable reason for anybody to write eval?.(arg). But that doesn't mean it should be a syntax error.

Quote myself from https://github.com/tc39/ecma262/issues/2062#issuecomment-651980690

The simplest thing from a programmer's perspective is to just threat ?. in a call as an orthogonal feature. Any special case treatment, including a syntax error for eval just adds to the already too large set of arbitrary special case rules that JavaScript programmers need to learn.

This basically a complexity budget issue. Don't make the language unnecessarily more complex by adding a nanny-style error. Better for linters to have a rule about eval?.()

This basically a complexity budget issue.

Thank you. As you appreciate, I love arguments from complexity budget. I agree that a SyntaxError here makes the spec and the formal language more complex. However, from the point of view of the programmer, coping with a system that silently deviates from programmer expectation is way more complex than coping with a syntax error.

I also agree that this is a dangerous argument if overused. The key here is that even sophisticated programmers, knowing all the rest of the language well, will arrive at different expectations. If one of these were clearly dominant over the other, once explained, I would probably advocate that. But if neither dominates, better to stop the lurking surprise before runtime.

I should also say that I don't really care about this one specifically. Approximately no one will write eval?.( on purpose. But it is a nice example to reinforce the precedent set by

-2**3

I will certainly not block consensus on this issue. Whatever everyone else can agree on is fine enough with me.

To me, the crux of -2**3 being a syntax error is _not_ that two people could disagree on how to interpret it, but rather that a _single_ person is led to interpret it differently depending on whitespace. The same argument applies to a || b ?? c .

eval?.()'s two possible interpretations, on the other hand, are reflective of two _separate_ camps:

  • Those favoring full orthogonality of ?. expect that eval?.() is exactly eval() when eval is non-nullish.
  • Those intimately familiar with (0,eval)() and kin may expect that direct eval is nothing other than eval(), that calling eval once it has passed through another operator is necessarily indirect.

We also had two separate camps surrounding how foo?.() itself should be interpreted. This meant either way having to instruct users via MDN and the like, but I doubt many folks would regret choosing one instead of scrapping optional call altogether. And I believe this edge case is far less controversial than that. 🙂

I doubt many folks would regret choosing one instead of scrapping optional call altogether.

[off topic] Yeah, I have to say I'm one of them. I've heard a programmer complain that he have to explained it 4 times to the reviewers for foo?.(x), and two times are for the same person 😅 I also have seen a bug which the author want foo?.[key] but accidently write foo?.(key).

Better for linters to have a rule about eval?.()

I may already comment about linters many times: every new linter rule have cost (development, documentation, discussion, adoption, configuration, deployment, etc.) in the whole js ecosystem, which have a much large scale than tc39, so the cost is much bigger than we realize.

Closing; we decided to leave the spec as-is in today's meeting.

So x |> eval will also be indirect. I feel it will surprise more people than eval?.() case...

i would not expect x |> eval() to be indirect, since that's sugar for the eval() appearing inside a function body. That should be discussed on the pipeline proposal.

It seems the meeting notes shows some delegates object direct eval because of x |> eval? Maybe I misunderstand.

since that's sugar for the eval()

But eval?.() could also be seen as sugar for eval != null ? eval() : undefined ...

@hax the objection is strictly to eval?.(x) but there is room for discussions wrt pipeline operator.

I would classify it as, making this one direct would force the pipeline case, as well as unknowable future cases, to all be direct as well; leaving this indirect still leaves the design space open.

In my opinion, we are just adding one and one ad-hoc rules which hard to infer and remember.

In my opinion, we are just adding one and one ad-hoc rules which hard to infer and remember.

I kinda agree. I had assumed from the meeting that we had settled this for all the new operators but it sounds like that's not the case?

But eval?.() could also be seen as sugar for eval != null ? eval() : undefined ...

That's not quite a general rule, if you replace eval with (eval = bar()) you don't evaluate bar() twice. That said, I doubt most people think of that nuance...

I had assumed from the meeting that we had settled this for all the new operators but it sounds like that's not the case?

That was my understanding. This sets precedence that new call syntaxes that invoke eval will be indirect evals. Only the literal source-text eval(…) can produce a direct eval.

My mistake, that seems fine to me too.

Was this page helpful?
0 / 5 - 0 ratings