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

Float numbers not pushed into array. #15427

Closed
SkeLLLa opened this issue Sep 15, 2017 · 11 comments
Closed

Float numbers not pushed into array. #15427

SkeLLLa opened this issue Sep 15, 2017 · 11 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@SkeLLLa
Copy link

SkeLLLa commented Sep 15, 2017

In two words: float numbers not pushed into array if in push function shift is called.

Version: 8.5.0
Platform: linux x86_64

class FixedArray extends Array {
  constructor(length) {
    super(length);
    this.maxLength = length;
  }

  push(...args) {
    console.log('what to push', args);
    super.push.apply(this, args);
    console.log('array after push', this);
    if (this.length > this.maxLength) {
      console.log('what is shifted?', super.shift.apply(this));
    }
  }
}

var ddd = new FixedArray(30);
ddd.push(1);
ddd.push(2);
ddd.push(3);
ddd.push(0.4);
ddd.push(0.5);
ddd.push(0.6);
console.log(ddd);

Result:

what to push [ 1 ]
array after push FixedArray [ <30 empty items>, 1, maxLength: 30 ]
what is shifted? undefined
what to push [ 2 ]
array after push FixedArray [ <29 empty items>, 1, 2, maxLength: 30 ]
what is shifted? undefined
what to push [ 3 ]
array after push FixedArray [ <28 empty items>, 1, 2, 3, maxLength: 30 ]
what is shifted? undefined
what to push [ 0.4 ]
array after push FixedArray [ <27 empty items>, 1, 2, 3, 0.4, maxLength: 30 ]
what is shifted? undefined
what to push [ 0.5 ]
array after push FixedArray [ <27 empty items>, 1, 2, 3, 0.5, maxLength: 30 ]
what is shifted? undefined
what to push [ 0.6 ]
array after push FixedArray [ <27 empty items>, 1, 2, 3, 0.6, maxLength: 30 ]
what is shifted? undefined
FixedArray [ <27 empty items>, 1, 2, 3, maxLength: 30 ]

Here's the code that stopped working in 8.5.0, but it was working in 8.4.0.

There's a class FixedArray that basically overrides push method. When I try to push float value into such array it pushes only once, however it should be pushed every second.

Another strange thing is if you change it to ddd.push(5); it works normally and integer value is pushed into this array every second.

If you remove shift() from a push, then all works also normally.

PS: In chrome versions from 60 to 63 all works normally.

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Sep 15, 2017
@vsemozhetbyt
Copy link
Contributor

Simplified case:

'use strict';

var ar = new Array(5);

ar.push(1);
console.log(ar);

ar.shift();
console.log(ar);

ar.push(2);
console.log(ar);

ar.shift();
console.log(ar);

ar.push(0.3);
console.log(ar);

ar.shift();
console.log(ar);
[ <5 empty items>, 1 ]
[ <4 empty items>, 1 ]
[ <4 empty items>, 1, 2 ]
[ <3 empty items>, 1, 2 ]
[ <3 empty items>, 1, 2, 0.3 ]
[ <3 empty items>, 1, 2 ]

@targos targos added the v8.x label Sep 15, 2017
@targos
Copy link
Member

targos commented Sep 15, 2017

This is fixed in V8 6.1.

@targos
Copy link
Member

targos commented Sep 15, 2017

/cc @nodejs/v8

@SkeLLLa
Copy link
Author

SkeLLLa commented Sep 15, 2017

@targos then this is related to #15393

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Sep 15, 2017

Even more simplified case:

var ar = [, , 0.1];

ar.shift();
console.log(ar);
[ <2 empty items> ]

So the issue is: Array.prototype.shift() skips empty slots till the first float number and shifts this number instead?

@SkeLLLa
Copy link
Author

SkeLLLa commented Sep 15, 2017

@vsemozhetbyt I don't think so, because is you look into my example there were some integers before floats and they were not shifted. Also:

var a = [,,,0.4,1,2,3,0.5];
a.shift()
console.log(a) // it will remove 0.5, not 0.4

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Sep 15, 2017

Yes, it seems for arrays with an initial hole and a non-SMI number, Array.prototype.shift() becomes Array.prototype.pop().

const a = [ , 2**31, 1 ];
a.shift();
console.log(a); // [ <1 empty item>, 2147483648 ]

@targos
Copy link
Member

targos commented Sep 15, 2017

The commit message here looks like it could be fixing this: v8/v8@81778aa

@andrasq
Copy link

andrasq commented Sep 15, 2017

a variant of Array.prototype.pop(), it pops but doesn't return the value (returns undefined)

@targos
Copy link
Member

targos commented Sep 18, 2017

v8/v8@81778aa was actually merged to V8 6.1 so this issue is fixed on master.

@bnoordhuis
Copy link
Member

v8.7.0 was released with V8 6.1, which has the fix. I'll close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

5 participants