Misc tests categorized as "pass", but are not valid programs

[bakkot]

• 970
• 995
• 1012

[RReverser]

Should I raise a new issue or report here?

pass/4de83a7417cd30dd.js contains /\u{110000}/u which denotes invalid Unicode character and thus should fail (as far as I know).

[bakkot]

Ah, thanks. Either way is fine. (That is indeed invalid - it's actually the only early error specified for regular expression literals.)

[RReverser]

As far as I can tell, these are also incorrectly marked as "pass":

• e0c3d30b6fe96812.js
• 8643da76fe7e95c7.js
• 6c4fe38464c16309.js
• 574ea84fc61bdc31.js
• 050a006ae573e260.js
• 2c0f785914da9d0b.js

In all these cases test assumes that 'use strict' inside the function doesn't affect the mode in which function name and arguments are checked, but in fact, it does.

[bakkot]

@RReverser, there's some disagreement about that, in fact.

The relevant early errors are here or here; it says "If the source code matching this production is strict mode code [...]", where "this production" is FunctionDeclaration : function BindingIdentifier ( FormalParameters ) { FunctionBody } or BindingIdentifier : Identifier (or whatever).

The only relevant part of the definition of "strict mode code" is "Function code is strict mode code if [...] the code that produces the value of the function's [[ECMAScriptCode]] internal slot begins with a Directive Prologue that contains a Use Strict Directive.".

And "function code" is "the source text that is parsed to supply the value of the [[ECMAScriptCode]] and [[FormalParameters]] internal slots (see 9.2) of an ECMAScript function object" - notably, not the BindingIdentifier.

As I read it, then, the production function BindingIdentifier ( FormalParameters ) { FunctionBody } is not strict mode code merely because its body contains a "use strict" directive; the parameters and body are, but not the whole production. As a consequence, that early error rule does not apply.

There's an old bugzilla bug about this somewhere, and I think raised it again a couple of years ago, but it's not been resolved. (No one cares all that much, I think.) I'm going to leave those tests as they are, since they match what the spec actually says.

[RReverser]

Well, I've just retested to make sure and SpiderMonkey (Firefox), V8 (Node.js, Chrome), JSC (Safari) and ChakraCore (Edge) all produce an error on such functions.

So in the best traditions of web standards 😄 , I would say if something is unclear for us when reading the spec but all engines agree, then this is indeed an error and should be tested as such.

[RReverser]

Also

"function code" is "the source text that is parsed to supply the value of the [[ECMAScriptCode]] and [[FormalParameters]] internal slots (see 9.2) of an ECMAScript function object"

BindingIdentifier does not matter here - this phrase just defines how to supply slots of the produced function. After we did fill [[ECMAScriptCode]], it is used to determine strictness pretty clearly:

Function code is strict mode code if [...] the code that produces the value of the function's [[ECMAScriptCode]] internal slot begins with a Directive Prologue that contains a Use Strict Directive.

So I would say engines implement spec correctly - first function is parsed to fill slots including [[ECMAScriptCode]], then [[ECMAScriptCode]] of the function is checked for a Use Strict Directive, then all the strictness rules start being applied by early errors.

[bakkot]

@RReverser: I know engines don't match the spec, and yes, it's probably a spec bug, but that's what it says! I'm hesitant to write tests against what the spec should be rather than what it is.

So I would say engines implement spec correctly

No, not as written. The definition of "function code" covers only the body and parameters, which means that the full FunctionDeclaration production, which also includes other code, is not "function code" of the function it is declaring, and as such is not made strict mode code by the presence of the "use strict" directive.

[RReverser]

The definition of "function code" covers only the body and parameters

Please re-read sequence above. Again, this definition does not matter for purpose of applying strict mode - it matters only for determining strictness. And, of course, to determine strictness, only function's body is used, not parameters or identifier, and after function body and parameters are set to slots, strictness is detected and is applied to entire FunctionDeclaration.

[RReverser]

To clarify again:

10.2.1

Function code is strict mode code if the associated FunctionDeclaration, FunctionExpression, GeneratorDeclaration, GeneratorExpression, AsyncFunctionDeclaration, AsyncFunctionExpression, MethodDefinition, ArrowFunction, or AsyncArrowFunction is contained in strict mode code or if the code that produces the value of the function's [[ECMAScriptCode]] internal slot begins with a Directive Prologue that contains a Use Strict Directive.

what is interesting here for us is

or if the code that produces the value of the function's [[ECMAScriptCode]] internal slot begins with a Directive Prologue that contains a Use Strict Directive

It's very explicit about strictness being determined using that slot.

And that slot is assigned as part of function parsing -> FunctionCreate -> FunctionInitialize from body of the function. Everything else doesn't matter for that particular slot, but after it's assigned, it's used for function code is strict mode code.

Now, for example

14.1.2

If the source code matching this production is strict mode code, it is a Syntax Error if BindingIdentifier is the IdentifierName eval or the IdentifierName arguments.

It says "code matching this production" (which is "function code") is "strict mode code" - it is "strict mode code" as 10.2.1 clearly defines.

I don't see any ambiguity here, and apparently neither does every other implementation.

[bakkot]

I don't understand the argument you're making.

I don't dispute that the function's body is strict mode code. Of course this is true. I dispute that the FunctionDeclaration production is strict mode code, on the grounds that the definition of "strict mode code" refers specifically to "function code", and that the definition of "function code" is explicit about including only the code parsed to produce the parameters and body.

That is, I agree the function code in question is strict, but I don't think the FunctionDeclaration production is itself part of said function code.

[RReverser]

I see what you're concerned with... you're saying that you're unsure that "code matching this production is strict mode code" means "function code is strict mode code", right?

But in that case other early errors from the same list that have exactly same wording wouldn't apply, for example:

If the source code matching this production is strict mode code, the Early Error rules for UniqueFormalParameters:FormalParameters are applied.

yet you do have test early/de9d9a6cd61407f5.js that checks that function parameters are unique when function has 'use strict' inside of its body - how is it different?

[bakkot]

@RReverser Yes, those are also at issue. Possibly I should move them. They're currently there because I confirmed with Allen that the early error for duplicate parameters is definitely intended to apply when the body has a "use strict" directive (i.e. it is definitely a spec bug) but we never seemed to get consensus that the early error for the function name is intended to apply when the body has a "use strict" directive (i.e. we do not have consensus that this is a spec bug).

Admittedly that's not that principled of a distinction, so, as I say, maybe I should recategorize the tests for duplicate parameters until we get the spec fixed.

[RReverser]

I see... this all feels just too vague, although intuitively seems connected. I agree it should be specified more clearly though.

[RReverser]

Just for the sake of thought experiment, trying again:

If the source code matching this production is strict mode code, it is a Syntax Error if BindingIdentifier is the IdentifierName eval or the IdentifierName arguments.

ok, what is "this production"?

FunctionDeclaration: function BindingIdentifier(FormalParameters){FunctionBody}
FunctionDeclaration: function (FormalParameters){FunctionBody}
FunctionExpression: function BindingIdentifier(FormalParameters){FunctionBody}

ok, now we need to determine if "this production" is "strict mode code":

Function code is strict mode code if the associated FunctionDeclaration, FunctionExpression, GeneratorDeclaration, GeneratorExpression, AsyncFunctionDeclaration, AsyncFunctionExpression, MethodDefinition, ArrowFunction, or AsyncArrowFunction is contained in strict mode code or if the code that produces the value of the function's [[ECMAScriptCode]] internal slot begins with a Directive Prologue that contains a Use Strict Directive.

So we have FunctionDeclaration / FunctionExpression on the left side of the link ("this production") and they're contained in the list on the right side of the link ("Function code is strict mode code if the associated FunctionDeclaration, FunctionExpression, ...").

So yeah, it's all clear on both sides apart from missing connector - whether "source code matching this production" is equivalent to "function code".

[bakkot]

Right. Except I don't think it's missing: "function code" is a defined term; it is defined to be "source text that is parsed to supply the value of the [[ECMAScriptCode]] and [[FormalParameters]] internal slots (see 9.2) of an ECMAScript function object".

So, it includes the source text for the body and for the parameters, but not e.g. not the name, which is definitely not "parsed to supply the value of the [[ECMAScriptCode]] and [[FormalParameters]] internal slots". This implies the full production isn't "function code".

[RReverser]

I think this is where I see uncertainty - it doesn't state that the matched code is limited to populating only [[ECMAScriptCode]] and [[FormalParameters]], just that it's used to populate these slots.

And indeed binding identifier doesn't have dedicated slot on function object, it's propagated as part of the lexical environment itself, so it couldn't fit into such definition of "function code" if it would be part of it.

Which is why I believe defining "function code" via slots it populates is bad, and should instead explicitly say something like "code that is parsed with grammar production FunctionDeclaration / FunctionExpression" which wouldn't raise any confusion or questions.

[RReverser]

Btw, what do test262 tests say about this? Is there test explicitly checking one or another behaviour?

[bakkot]

I didn't think so, but looking again, it looks like it's testing that it's an error, though I can't find a test not using eval.

These two projects should definitely agree. (And in fact I've been meaning to import the tests from test262, though I have to figure out a good way to deal with license information.) I've opened #13 to track resolving the inconsistency; the fix is probably going to be updating the spec and this repo.

[RReverser]

@bakkot Thanks. Guess this needs to be pinged upstream (again) for someone to take a look. Honestly I don't care which way will be the correct one as long as it's clearly specified and engines would do the same.

[bakkot]

If I have time this weekend I'll try to write some spec text and ask for consensus at the TC39 meeting this week, but it'll probably have to wait.

[RReverser]

Reading further (digging into ES5 / ES5.1 specs) I tend to agree that the spec text suggests that "use strict" in the body should not affect binding of function name to the lexical environment, but then it's even more annoying that all the engines disagree.

[bterlson]

I can bring this up in committee next week. I don't really have an opinion on which is better, so am fine with amending the spec to say the strictness also applies to the function binding.

[michaelficarra]

I would prefer what I understand to be the current spec text, which is that the binding is not considered strict mode code. It's already strange enough that parameter lists are strict mode code.

[RReverser]

It's already strange enough that parameter lists are strict mode code.

Not really strange if you consider the fact that you need to use them inside of the function. If function has "use strict" at the beginning, you otherwise would be able to declare parameters with reserved names but not use them inside of the very same function, which would be really weird.

[bterlson]

@michaelficarra: given that parameter lists are strict mode code, it seems strange that other parts of the production are not strict. As I see it :)

[bakkot]

@bterlson, I dunno, I find mike's argument here persuasive:

the identifier static is used to create an entry in the outer (sloppy) environment record. The strictness of the function has no effect here.

[bakkot]

Closing this in favor of #13; the rest have been resolved.