new Date error solved

[LeeHayun]

I solved the problem mentioned in issue #1386 .
(new Date() fails for any date in October on leap years)
The issue was raised long time before,
but the problem was not solved for a long time.

Therefore, i solved the problem, and
In addition, i enhanced the algorithm. (Change binary search to simple algorithm)
The algorithm uses ecma standard api.

I did two unit tests (bug fix, performance).
Following is code is used for unit tests on IoT.js.

fail_count=0;
for (y=1950; y<2050; y++) {
  for (m=0; m<12; m++) {
    for (d=1; d<32; d++) {
      var date = new Date(y, m, d);
      if (isNaN(date)) {
        fail_count++;
      }
    }
  }
}
console.log(fail_count);

var t0 = new Date().getTime();
for (y=1950; y<2050; y++) {
  for (m=0; m<12; m++) {
    for (d=1; d<32; d++) {
      var date = new Date(y, m, d);
    }
  }
}
var t1 = new Date().getTime();
var delay = t1 - t0;
console.log(delay + "ms");

This table is results of unit tests.

/	Original JerryScript	Modified JerryScript
fail_count	775	0
delay (ms)	33275	16823

[LaszloLango]

Something isn't right with your branch. Please rebase to the current mater and add unit test to it.

[LeeHayun]

@LaszloLango I modified PR and PR message according to your request. Please review about this.

[LaszloLango]

@LeeHayun, I'm still missing the test from the PR. Please add it to your commit too, not only to the description. Add a test to tests/unit/test-date-helpers.c or to tests/jerry/regression-test-issue-1386.js. Both would be the best. The commit message should be improved too. Please add the related issue number to the commit message like "Fixes #1386" and add a bit more detailed description.

[LeeHayun]

@LaszloLango I added unit-test code, and regression-test code as you said.
Javascript code which i wrote above is just for testing by myself, so please never mind.
In addition, i modified my commit message as you said. (e.g., Fixes #1386 ~ )
Please check it out. Thanks.

[LaszloLango]

Different dates must not have the same day value.

[esevan]

@LaszloLango ecma_data_make_day (2016, 8, 31) must be same as ecma_data_make_day (2016,9,1) since there's no 2016/9/31 so that it's regarded as 2016/10/1.

[LaszloLango]

My bad, both values means October 1 of 2016.

[zherczeg]

Shouldn't this be an increasing number? So get the value of 1950/01/01, and the next one should be bigger by one, the next is bigger by two, etc?

var last_date = new Date(1950, 01, 01);
for (...) {
   last_date++;
   assert (new Date(y, m, d) == last_date)
}

[LeeHayun]

@zherczeg The day test loop is from 1 to 31 so that it wouldn't be incremented on such date like (2/31->3/1). Moreover, ecma_date_make_day() returns NaN when time value is not equal to relative day from (y,m,1). This function implemented by ECMA-262 v5, 15.9.1.12 So, step 7 in this link guarantees that the date is incremented by one if it's correct.

[zherczeg]

I think we should find a way to test this since the original problem was arised from untested values.

What about reset the counter when d = 1 (before the d loop) ?

[LeeHayun]

@zherczeg I modified regression test code according to your request.

In addition, I found a bug in ecma_date_week_day(). This function returns minus value when return week_day before 1970 years. For example, value of new Date(1950, 1, 1).getDay() is -4. But, in ECMA standard, 15.9.1.6 defines 'WeekDay(t) = (Day(t) + 4) modulo 7' and 5.2 describes 'The notation “x modulo y” (y must be finite and nonzero) computes a value k of the same sign as y (or zero) ...' for modulo operation.

So, I fixed ecma_date_week_day() to comply with standard. Please check it.

Additionally, Travis server doesn't work for a long time.

[zherczeg]

Excellent. This is why it is worth writing complex tests :)

A question: was it intentional not following jerry coding style in the test case?

[LeeHayun]

@zherczeg Thank you. Can you tell me detailedly what part not follows jerry coding style?

I modified that code according to the the other code's style.

[zherczeg]

Please put a space before the open bracket. Same for all function calls.

[LeeHayun]

@zherczeg I put a space before the open bracket for all function calls. Please check it.

[zherczeg]

LGTM. Nice improvement.

[LaszloLango]

LGTM