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: Introduce IsStrict() abstract operation #3209

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Oct 27, 2023

I've broken it out into 6 5 commits in case that helps review.


commit 1:

Rename the existing SDO "IsStrict" to "ScriptIsStrict", because it's specific to Scripts, and I want the general name for the general operation.


commit 2:

The algorithms in the spec have 7 occurrences of:

X is contained in strict mode code

and 22 occurrences of:

X is strict mode code

This commit changes all of the former to the latter.

It isn't obvious whether or not this is a trivial change.

At first, I thought it was non-trivial, with the following reasoning: Clearly, you can have islands of strict mode code (SMC) within a sea of non-SMC. So, for such an island, it seems like it is SMC, but it's not contained in SMC, it's contained in non-SMC. (And so, this commit's change might cause the conditions to admit cases they currently don't, when X is SMC but isn't contained in SMC.)

However, consider test262's language/statements/function/name-eval-strict-body.js, which is basically:

/*---
negative
  phase: parse
  type: SyntaxError
flags: [noStrict]
---*/
$DONOTEVALUATE();
function eval() { 'use strict'; }

which throws a SyntaxError because (roughly speaking) 'eval' isn't allowed as the name of a strict mode function.

In more detail:

  • The [noStrict] means that the global code is non-SMC.

  • The 'use strict' means that the source text matched by:

    • the FunctionBody,
    • the (empty) FormalParameters, and
    • the BindingIdentifier

    are all SMC.

  • The relevant early error rule is from 13.1.1 Static Semantics: Early Errors, for BindingIdentifier : Identifier:

It is a Syntax Error if the source text matched by this production is contained in strict mode code and the StringValue of |Identifier| is either "arguments" or "eval".

So in order for this error to apply, we must conclude that the source text matched by BindingIdentifier is contained in SMC. We can readily agree that it is SMC, but "contained in" is trickier: is eval contained in eval? Or maybe the idea is that eval is part of all the SMC associated with the function decl.

Anyhow, it seems like, as far as the spec is concerned, "is contained in SMC" means the same as "is SMC",


commit #3:

Normally, we say "the source text matched by X is strict mode code", but 3 spots leave out "the source text matched by". I think that's just a mistake, so this commit inserts the missing text.


commit #4:

With the relevant pseudocode mostly consistified, introduce the IsStrict() abstract operation.


commit #5:

Change:

If IsStrict(X) is *true*, let _strict_ be *true*; else let _strict_ be *false*.

to just:

Let _strict_ be IsStrict(X).

commit #6: [Dropped Oct 28]

IsStrict(this production)
sounds odd, like it's asking whether a production is strict. So this commit changes such usages to
IsStrict(this |Foo|)
where possible (i.e., when the associated productions have only one LHS symbol), and to
IsStrict(this phrase)
otherwise.

spec.html Outdated Show resolved Hide resolved
@michaelficarra
Copy link
Member

I like this mainly for having a good way to find the locations where strict mode code is being tested. This should establish the editorial convention that we use IsStrict wherever applicable. This should also establish the editorial convention that we refer to a production by the LHS nonterminal in the SDO steps when the SDO is defined over a single production (or productions all sharing a LHS).

spec.html Outdated Show resolved Hide resolved
@jmdyck
Copy link
Collaborator Author

jmdyck commented Oct 29, 2023

force-pushed to drop the last commit, as it's been superseded by PR #3210.

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Mar 20, 2024
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Mar 20, 2024
(The pre-existing "IsStrict" AO is renamed to "ScriptIsStrict".)
(The pre-existing "IsStrict" AO is renamed to "ScriptIsStrict".)
@ljharb ljharb merged commit 80c4419 into tc39:main Mar 20, 2024
7 checks passed
@jmdyck jmdyck deleted the strict_mode branch March 21, 2024 20:26
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.

None yet

5 participants