-
Notifications
You must be signed in to change notification settings - Fork 425
Conversation
As this change sits on top of, but also includes, previously approved pull requests, it's difficult for me to see what's new. Can you merge the other pull requests and rebase this one on top of them? |
My pull requests always involve only the last commit. |
@@ -74,7 +74,7 @@ export function getPureBinaryOperationResultType( | |||
} else if (op === ">>>" || op === "<<" || op === ">>" || op === "&" || op === "|" || op === "^" || | |||
op === "**" || op === "%" || op === "/" || op === "*" || op === "-") { | |||
return reportErrorIfNotPure(IsToNumberPure, NumberValue); | |||
} else if (op === "in") { | |||
} else if (op === "in" || op === "instanceof") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are plain function values simple? Are bound function values simple? If so, I think the code below would have to peek into .$BoundTargetFunction
to figure out if this operation is safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll make the isSimple check safer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that it is perfectly safe already. Far too safe, in fact. But making a better alternative to isSimple is best left to a later pull request.
@hermanventer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Now give error messages for instanceof operators when the right operand is too abstract.