Skip to content

Possible bug: instanceof evaluates to true in Node 4 but not Node 6 #7592

Closed
Closed
Possible bug: instanceof evaluates to true in Node 4 but not Node 6#7592
Labels
v8 engineIssues and PRs related to the V8 dependency.

Description

@ghost
ghost
opened on Jul 7, 2016
  • Version: 6.3.0:
  • Platform: Darwin 15.5.0 Darwin Kernel Version 15.5.0: Tue Apr 19 18:36:36 PDT 2016; root:xnu-3248.50.21~8/RELEASE_X86_64 x86_64:
  • Subsystem:

In Node 4, the last statement evaluates to true, but in Node 6 it evaluates to false. Is this a bug?

F = () => {};
F.prototype = {};
Object.create(F.prototype) instanceof F;

Another user verified the behavior on their Windows system: http://stackoverflow.com/questions/38253656/instanceof-evaluates-to-true-in-node-4-but-not-node-6

Activity

  1. mscdex
    added
    v8 engineIssues and PRs related to the V8 dependency.
    on Jul 7, 2016

  2. mscdex commented on Jul 7, 2016

    @mscdex
    mscdex
    on Jul 7, 2016
    Contributor

    /cc @nodejs/v8

  3. mscdex commented on Jul 7, 2016

    @mscdex
    mscdex
    on Jul 7, 2016
    Contributor

    I should note that it returns true in master.

  4. bnoordhuis commented on Jul 7, 2016

    @bnoordhuis
    bnoordhuis
    on Jul 7, 2016
    Member

    Looks like a bug to me. v7.0.0-pre (V8 5.1) returns true, which is what I would expect.

  5. Trott commented on Jul 7, 2016

    @Trott
    Trott
    on Jul 7, 2016
    Member

    All Node.js version 6.x releases that I tried returned the surprising false including 6.0.0.

    Version 5.12.0 returned true.

    So this would seem to be either a bug in the V8 that shipped with Node.js v6.x or (less likely, I imagine, but still possible) a bug in a patch that we are only floating in v6.x. (I'm not even sure there are any that are in 6.x but not in 5.x or current master.)

  6. fhinkel commented on Jul 7, 2016

    @fhinkel
    fhinkel
    on Jul 7, 2016
    Contributor

    In d8 version 5.0.71.54 it returns false. So not related to floating patches.

  7. fhinkel commented on Jul 8, 2016

    @fhinkel
    fhinkel
    on Jul 8, 2016
    Contributor

    This is the commit that fixed it: https://codereview.chromium.org/1810953002/

    The patch applies clean on v6.x and fixes this issue. Do we want to float the patch on v6.x or just wait because it's fixed in V8 5.1 anyways?

  8. Trott commented on Jul 8, 2016

    @Trott
    Trott
    on Jul 8, 2016
    Member

    /cc @nodejs/lts Not strictly an LTS question/issue, but I suspect they'll have an opinion on @fhinkel's question.

  9. MylesBorins commented on Jul 8, 2016

    @MylesBorins
    MylesBorins
    on Jul 8, 2016
    Contributor

    Another patch was recently back ported to 5.1 by @bnoordhuis

    There is going to be a discussion about maintaining lts release streams directly from V8 to avoid floating patches. Aiming for 6 am estwif anyone wants to join.

  10. 21 remaining items

  11. evanlucas
    added 3 commits that reference this issue on Jul 21, 2016

    deps: cherry-pick 3a903c4 for PPC from V8 upstream

    e5cce7a

    deps: cherry-pick 5b5d24b for X87 from V8 upstream

    2870061

    test: add regression test for instanceof

    44c9a72

  12. fhinkel commented on Jul 22, 2016

    @fhinkel
    fhinkel
    on Jul 22, 2016
    Contributor

    Fixed in #7638.

    Can we close this?

  13. addaleax commented on Jul 22, 2016

    @addaleax
    addaleax
    on Jul 22, 2016
    Member

    Yup, works as expected in 6.3.1. Thanks for taking care of this!

  14. addaleax
    closed this as completedon Jul 22, 2016
  15. BethGriggs
    added 3 commits that reference this issue on Jul 22, 2016

    deps: cherry-pick 2aa070be from V8 upstream

    7963103

    deps: cherry-pick 3a903c4 for PPC from V8 upstream

    3d4d0db

    deps: cherry-pick 5b5d24b for X87 from V8 upstream

    1521a0c
  16. richardlau
    added a commit that references this issue on Aug 15, 2016

    deps: cherry-pick 71d525a for S390 from V8 upstream

    d051e78
  17. jasnell
    added a commit that references this issue on Oct 20, 2016

    test: add regression test for instanceof

    d62e7bd
  18. MylesBorins
    added a commit that references this issue on Nov 15, 2016

    test: add regression test for instanceof

    a038fcc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    v8 engineIssues and PRs related to the V8 dependency.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions