[Webkit-unassigned] [Bug 166665] Super property access in base class constructor doesn't work

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 17 12:47:56 PST 2017


https://bugs.webkit.org/show_bug.cgi?id=166665

--- Comment #8 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 299045
  --> https://bugs.webkit.org/attachment.cgi?id=299045
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299045&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Current patch fixed allow to use super inside of the constructor for classes 

Nit: "Current patch fixed allow" -> "Allow".

You should also explain how you're fixing the problem.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3696
> +        } else if (metadata->superBinding() == SuperBinding::Needed)
> +            superIsUsedInConstructor = true;

Why don't we just create a new boolean like "needsHomeObject" and set it to true whenever this is true or m_classHeritage is set?

> Source/JavaScriptCore/parser/Parser.h:1605
> +    ALWAYS_INLINE SuperBinding getSuperBindingForConstructor(ConstructorKind constructorKind, SuperBinding superBinding, bool needsSuperBinding, bool currentScopeUsesEval, InnerArrowFunctionCodeFeatures innerArrowFunctionFeatures)

Nit: we don't prefix a getter function with "get" unless there is an out argument.
See https://webkit.org/code-style-guidelines/#names-setter-getter

> Source/JavaScriptCore/parser/Parser.h:1612
> +            methodSuperBinding = (needsSuperBinding || isSuperUsedInInnerArrowFunction || isEvalUsedInInnerArrowFunctions || currentScopeUsesEval) ? SuperBinding::Needed : SuperBinding::NotNeeded;

Why do we need super binding when eval is used inside an arrow function?
I don't see any test case of that either.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170117/b1cf6f14/attachment.html>


More information about the webkit-unassigned mailing list