Skip to content

Commit

Permalink
[MERGE #5712 @wyrichte] Fixes #5701 - Functions declared as methods d…
Browse files Browse the repository at this point in the history
…efine a `prototype` property

Merge pull request #5712 from wyrichte:build/wyrichte/bug_5701_1

ES19 requires that, unless otherwise stated, all methods should have a prototype. ChakraCore adds prototypes to methods defined in objects. In order to not allow prototypes to be added to methods defined in objects, the ErrorOnNew flag is atted to the pNodeFnc's FuncInfo's attributes. Class constructors are methods but are defined to have prototypes, so they are not given the ErrorOnNew flag.
  • Loading branch information
wyrichte committed Sep 21, 2018
2 parents 6efd6f6 + ee2c09b commit e1583ae
Show file tree
Hide file tree
Showing 10 changed files with 193 additions and 132 deletions.
8 changes: 8 additions & 0 deletions lib/Runtime/ByteCode/ByteCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,14 @@ static Js::FunctionInfo::Attributes GetFunctionInfoAttributes(ParseNodeFnc * pno
if (pnodeFnc->IsMethod())
{
attributes = (Js::FunctionInfo::Attributes)(attributes | Js::FunctionInfo::Attributes::Method);

// #sec-runtime-semantics-classdefinitionevaluation calls #sec-makeconstructor. #sec-makeconstructor
// creates a prototype. Thus a method that is a class constructor has a prototype and should not
// throw an error when new is called on the method.
if (!pnodeFnc->IsClassConstructor())
{
attributes = (Js::FunctionInfo::Attributes)(attributes | Js::FunctionInfo::Attributes::ErrorOnNew);
}
}
if (pnodeFnc->IsGenerator())
{
Expand Down
64 changes: 32 additions & 32 deletions lib/Runtime/Library/InJavascript/Intl.js.bc.32b.h

Large diffs are not rendered by default.

64 changes: 32 additions & 32 deletions lib/Runtime/Library/InJavascript/Intl.js.bc.64b.h

Large diffs are not rendered by default.

64 changes: 32 additions & 32 deletions lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.32b.h

Large diffs are not rendered by default.

64 changes: 32 additions & 32 deletions lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.64b.h

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/AsmJs/bug16252562.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ bug16252562.js(9, 3)
Unhandled parse opcode for asm.js

Asm.js compilation failed.
new.target: Bar() {console.log(`new.target: ${new.target}`);}
new.target: function () { console.log(`new.target: ${new.target}`); }
2 changes: 1 addition & 1 deletion test/AsmJs/bug16252562.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ function AsmModule(stdlib, foreign) {
}
return foo;
}
AsmModule(this, {Bar() {console.log(`new.target: ${new.target}`);}})();
AsmModule(this, { Bar: function () { console.log(`new.target: ${new.target}`); } })();
41 changes: 41 additions & 0 deletions test/Prototypes/NoPrototypeForMethod.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

var failed = false;

var o = {
f() { }
}

if (o.f.hasOwnProperty("prototype")) {
failed = true;
WScript.Echo("Failed: a method within an object literal should not have a prototype");
}

try {
new o.f();
failed = true;
WScript.Echo("Failed: a method within an object literal should not have a prototype and thus new should not be valid");
}
catch (e) {
}

class C {
f() { }
}

if (new C().f.hasOwnProperty("prototype")) {
failed = true;
WScript.Echo("Failed: a method within a class should not have a prototype");
}

if (!(new C().constructor.hasOwnProperty("prototype"))) {
failed = true;
WScript.Echo("Failed: a class' constructor should have a prototype");
}

if (!failed) {
WScript.Echo("Pass");
}
5 changes: 5 additions & 0 deletions test/Prototypes/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,9 @@
<files>shadow2.js</files>
</default>
</test>
<test>
<default>
<files>NoPrototypeForMethod.js</files>
</default>
</test>
</regress-exe>
11 changes: 9 additions & 2 deletions test/es6/proxyconstruction.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,19 @@ var tests = [
}
},
{
name: "Proxy of Object literal method should be constructable",
name: "Proxy of Object literal function should be constructable",
body() {
let testingMethod = {test() {}}
let testingMethod = {test:function() {}}
testConstructProxy(testingMethod.test, false);
}
},
{
name: "Proxy of Object literal method should not be constructable",
body() {
let testingMethod = { test() { } }
testConstructProxy(testingMethod.test, true);
}
},
{
name: "Proxy of Date object should be constructable",
body() {
Expand Down

0 comments on commit e1583ae

Please sign in to comment.