Prevent calls to super outside of a constructor #2452
Conversation
ChadKillingsworth
requested review from
blickly
and removed request for
blickly
| @@ -58,6 +58,10 @@ | |||
| "JSC_CANNOT_CONVERT_YET", | |||
| "ES6 transpilation of ''{0}'' is not yet implemented."); | |||
|
|
|||
| static final DiagnosticType INVALID_SUPER = | |||
There was a problem hiding this comment.
Could you rename this to INVALID_SUPER_CALL?
There was a problem hiding this comment.
@brad4d Fixed
ChadKillingsworth
force-pushed
the
super-calls-online-in-constructor
branch
from
cc8e8c3
to
077ab55
Compare
|
@ChadKillingsworth sorry, but @MatrixFrog refactored class transpilation out of |
|
I thought I'd checked that this wouldn't cause conflicts. Sorry!
|
ChadKillingsworth
force-pushed
the
super-calls-online-in-constructor
branch
from
077ab55
to
960f150
Compare
|
All fixed - it barely conflicted |
ChadKillingsworth
force-pushed
the
super-calls-online-in-constructor
branch
from
960f150
to
c3eed57
Compare
|
Sent for internal testing and review. |
|
|
||
| if (exprRoot.getParent().isNew()) { | ||
| compiler.report(JSError.make(node, INVALID_SUPER_CALL)); | ||
| } else if (NodeUtil.isCallOrNewTarget(exprRoot) |
There was a problem hiding this comment.
why isCallOrNewTarget() here?
It's redundant with the previous if.
| potentialCallee = parent; | ||
| Node exprRoot = node; | ||
|
|
||
| if (exprRoot.getParent().isGetElem() || exprRoot.getParent().isGetProp()) { |
There was a problem hiding this comment.
An internal test system found that changing this to be if (true) { didn't cause any test failures.
This implies a missing test and made me realize it's not as clear as it should be what's happening here.
Could this logic be clarified to something like this?
private void visitSuper(Node superNode, Node parent) {
checkState(superNode.isSuper());
if (parent.isCall()) {
// super(...)
visitBareSuperCall(superNode, parent);
} else if (parent.isGetProp()) {
checkState(parent.getFirstChild() == superNode);
// super.something
if (parent.getParent().isCall()) {
// super.something(...)
visitSuperDotSomethingCall(superNode, parent);
} else {
// super.something used in some other way
// report not supported yet error
}
} else if (parent.isGetElem()) {
checkState(superNode == parent.getFirstChild());
// super[...]
// report not supported yet error
} else if (parent.isNew()) {
// new super(...)
// invalid use of super
} else {
// some other use of super we don't support yet
}
}
|
ping @ChadKillingsworth |
|
@brad4d I've been slammed - I hope to get back to this soon. |
|
@ChadKillingsworth |
|
@brad4d Changes made. |
|
@ChadKillingsworth |
|
test results look good |
Closes google#2452 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=157253710
Prerequisite for #2418
Emit errors for calls to
superoutside of a constructor.