Do not use BigInt literal without features: [BigInt]

[Constellation]

typeCoercion.js is included in several tests which are not marked as features: [BigInt].
Since BigInt is new feature, we should not make the above unrelated tests failed due to
lack of BigInt syntax support.

Close #1252.

[leobalter]

#1252 (comment)

Can you point any test including typeCoercion.js and not adding a feature tag for BigInt? This would be a linter problem we also need to solve.

I'm not sure replacing the literal value by the constructor is the best solution if neither are implemented. Anyway, I'm inclined to use this proposed change to at least avoid a parsing error. My only blocking question is: should we allow the test runner to fail at the end (using BigInt ctor) or early (using the literal form)?

Maybe the constructor allows a better handling for the unsupported feature and reading the features tags, but I'm afraid this might open a path for a bad practice.

I'll revisit this issue on the week days.

[Constellation]

Oops, sorry, test/built-ins/String/prototype/indexOf/position-tointeger.js includes feature flag BigInt since it includes this file.

But I think this file should not require BigInt because this test is not using it.
Actually, this test is not marked as BigInt feature when this test is created at 29938e9.

In addition, some places of typeCoercion.js take care of the environments missing BigInt. For example,
https://github.com/Constellation/test262/blob/d2724b9f438cbd79789bb2c41cdd219944e48217/harness/typeCoercion.js#L234-L237 is,

  if (typeof BigInt !== "undefined") {
    // ToNumber: BigInt -> TypeError
    testPrimitiveValue(BigInt(0));
  }

Since typeCoercion.js is a set of utilities to test type coercion, I expect that we can replace existing type coercion tests with this file's utility. But I don't think it is good that making these tests failed if we do not support BigInt. This is because these tests are not for BigInt.

So, I think we need to take care of using new syntax feature (like BigInt literal) in a utitlty file.
I think either (1) using typeof BigInt check when using BigInt in typeCoercion.js, or (2) separating BigInt feature from typeCoercion.js is better.

[leobalter]

This is accurate. I’ll give a round check for more improvements. We can start with a type checking for the constructor but if this trigger other issues we should have a separate file, indeed.

…

On Tue, Oct 3, 2017 at 5:33 AM Yusuke Suzuki ***@***.***> wrote: Oops, sorry, test/built-ins/String/prototype/indexOf/position-tointeger.js includes feature flag BigInt since it includes this file. But I think this file should not require BigInt because this test is not using it. Actually, this test is not marked as BigInt feature when this test is created at 29938e9 <29938e9> . In addition, some places of typeCoercion.js take care of the environments missing BigInt. For example, https://github.com/Constellation/test262/blob/d2724b9f438cbd79789bb2c41cdd219944e48217/harness/typeCoercion.js#L234-L237 is, if (typeof BigInt !== "undefined") { // ToNumber: BigInt -> TypeError testPrimitiveValue(BigInt(0)); } Since typeCoercion.js is a set of utilities to test type coercion, I expect that we can replace existing type coercion tests with this file's utility. But I don't think it is good that making these tests failed if we do not support BigInt. This is because these tests are not for BigInt. So, I think we need to take care of using new syntax feature (like BigInt literal) in a utitlty file. I think either (1) using typeof BigInt check when using BigInt in typeCoercion.js, or (2) separating BigInt feature from typeCoercion.js is better. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1253 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASYkUJUsXkpB2rYMrpnCOj15niPiFHTks5sof9igaJpZM4Ppgjb> .

[Constellation]

Thank you :D