Remove getCalleeClosure

Summary:
`getCalleeClosure` seems prone to being used incorrectly. Most places
that call `getCalleeClosure` immediately `vmcast` the result to some
subclass of `Callable`, so we should just replace those calls with
`getCalleeClosureUnsafe`, which is clear that the operation is unsafe
and saves us a branch. Some other places immediately feed the result
into `dyn_vmcast_or_null`, which also adds an extra branch.

Given that, this diff removes `getCalleeClosure` entirely, and makes
the behaviour at the call sites more explicit.

Reviewed By: tmikov

Differential Revision: D30918025

fbshipit-source-id: 64dbe7f6c942482c69616d3a2cc013e4cf629488

include/hermes/VM/StackFrame-inline.h

@@ -38,11 +38,6 @@ StackFramePtrT<isConst>::getCalleeCodeBlock() const {
   }
 }
 
-template <bool isConst>
-inline Callable *StackFramePtrT<isConst>::getCalleeClosure() const {
-  return dyn_vmcast<Callable>(getCalleeClosureOrCBRef());
-}
-
 template <bool isConst>
 inline Handle<Environment> StackFramePtrT<isConst>::getDebugEnvironmentHandle()
     const {

include/hermes/VM/StackFrame.h

@@ -179,10 +179,6 @@ class StackFramePtrT {
   /// CodeBlock *.
   inline Handle<Callable> getCalleeClosureHandleUnsafe() const;
 
-  /// \return a pointer to the callee closure, if we have it, or nullptr
-  ///   if it is a CodeBlock.
-  Callable *getCalleeClosure() const;
-
   /// \return the callee's CodeBlock, i.e. the CodeBlock that is executing in
   ///   this frame. It could be nullptr if calleeClosure is a Callable but not
   ///   a JSFunction.

lib/VM/Interpreter-slowpaths.cpp

@@ -25,7 +25,8 @@ void Interpreter::saveGenerator(
     Runtime *runtime,
     PinnedHermesValue *frameRegs,
     const Inst *resumeIP) {
-  auto *innerFn = vmcast<GeneratorInnerFunction>(FRAME.getCalleeClosure());
+  auto *innerFn =
+      vmcast<GeneratorInnerFunction>(FRAME.getCalleeClosureUnsafe());
   innerFn->saveStack(runtime);
   innerFn->setNextIP(resumeIP);
   innerFn->setState(GeneratorInnerFunction::State::SuspendedYield);

lib/VM/Interpreter.cpp

@@ -156,7 +156,7 @@ CallResult<PseudoHandle<JSGenerator>> Interpreter::createGenerator_RJS(
   }
 
   auto generatorFunction = runtime->makeHandle(vmcast<JSGeneratorFunction>(
-      runtime->getCurrentFrame().getCalleeClosure()));
+      runtime->getCurrentFrame().getCalleeClosureUnsafe()));
 
   auto prototypeProp = JSObject::getNamed_RJS(
       generatorFunction,
@@ -1730,7 +1730,7 @@ CallResult<HermesValue> Interpreter::interpretFunction(
 
       CASE(CompleteGenerator) {
         auto *innerFn = vmcast<GeneratorInnerFunction>(
-            runtime->getCurrentFrame().getCalleeClosure());
+            runtime->getCurrentFrame().getCalleeClosureUnsafe());
         innerFn->setState(GeneratorInnerFunction::State::Completed);
         ip = NEXTINST(CompleteGenerator);
         DISPATCH;
@@ -1751,7 +1751,7 @@ CallResult<HermesValue> Interpreter::interpretFunction(
 
       CASE(StartGenerator) {
         auto *innerFn = vmcast<GeneratorInnerFunction>(
-            runtime->getCurrentFrame().getCalleeClosure());
+            runtime->getCurrentFrame().getCalleeClosureUnsafe());
         if (innerFn->getState() ==
             GeneratorInnerFunction::State::SuspendedStart) {
           nextIP = NEXTINST(StartGenerator);
@@ -1766,7 +1766,7 @@ CallResult<HermesValue> Interpreter::interpretFunction(
 
       CASE(ResumeGenerator) {
         auto *innerFn = vmcast<GeneratorInnerFunction>(
-            runtime->getCurrentFrame().getCalleeClosure());
+            runtime->getCurrentFrame().getCalleeClosureUnsafe());
         O1REG(ResumeGenerator) = innerFn->getResult().unboxToHV(runtime);
         O2REG(ResumeGenerator) = HermesValue::encodeBoolValue(
             innerFn->getAction() == GeneratorInnerFunction::Action::Return);

lib/VM/JSLib/HermesBuiltin.cpp

@@ -232,8 +232,8 @@ hermesBuiltinThrowTypeError(void *, Runtime *runtime, NativeArgs args) {
 /// \return `undefined`
 CallResult<HermesValue>
 hermesBuiltinGeneratorSetDelegated(void *, Runtime *runtime, NativeArgs args) {
-  auto *gen = dyn_vmcast_or_null<GeneratorInnerFunction>(
-      runtime->getCurrentFrame().getPreviousFrame().getCalleeClosure());
+  auto *gen = dyn_vmcast<GeneratorInnerFunction>(
+      runtime->getCurrentFrame().getPreviousFrame().getCalleeClosureOrCBRef());
   if (!gen) {
     return runtime->raiseTypeError(
         "generatorSetDelegated can only be called as part of yield*");

lib/VM/JSLib/Intl.cpp

@@ -679,10 +679,10 @@ intlCollatorSupportedLocalesOf(void *, Runtime *runtime, NativeArgs args) {
 
 CallResult<HermesValue>
 intlCollatorCompare(void *, Runtime *runtime, NativeArgs args) {
-  PseudoHandle<Callable> cc =
-      createPseudoHandle(runtime->getCurrentFrame()->getCalleeClosure());
+  auto *nf = vmcast<NativeFunction>(
+      runtime->getCurrentFrame()->getCalleeClosureUnsafe());
   PseudoHandle<DecoratedObject> collatorHandle =
-      getCollator(PseudoHandle<NativeFunction>::vmcast(std::move(cc)), runtime);
+      getCollator(createPseudoHandle(nf), runtime);
 
   // Since collatorHandle came out of an internal slot, it's an
   // assertable failure if it has the wrong type.
@@ -967,10 +967,10 @@ CallResult<double> dateNowValue(Runtime *runtime, NativeArgs args) {
 
 CallResult<HermesValue>
 intlDateTimeFormatFormat(void *, Runtime *runtime, NativeArgs args) {
-  PseudoHandle<Callable> cc =
-      createPseudoHandle(runtime->getCurrentFrame()->getCalleeClosure());
-  PseudoHandle<DecoratedObject> dateTimeFormatHandle = getDateTimeFormat(
-      PseudoHandle<NativeFunction>::vmcast(std::move(cc)), runtime);
+  auto *nf = vmcast<NativeFunction>(
+      runtime->getCurrentFrame()->getCalleeClosureUnsafe());
+  PseudoHandle<DecoratedObject> dateTimeFormatHandle =
+      getDateTimeFormat(createPseudoHandle(nf), runtime);
 
   // Since dateTimeFormatHandle came out of an internal slot, it's an
   // assertable failure if it has the wrong type.
@@ -1247,10 +1247,10 @@ intlNumberFormatSupportedLocalesOf(void *, Runtime *runtime, NativeArgs args) {
 
 CallResult<HermesValue>
 intlNumberFormatFormat(void *, Runtime *runtime, NativeArgs args) {
-  PseudoHandle<Callable> cc =
-      createPseudoHandle(runtime->getCurrentFrame()->getCalleeClosure());
-  PseudoHandle<DecoratedObject> numberFormatHandle = getNumberFormat(
-      PseudoHandle<NativeFunction>::vmcast(std::move(cc)), runtime);
+  auto *nf = vmcast<NativeFunction>(
+      runtime->getCurrentFrame()->getCalleeClosureUnsafe());
+  PseudoHandle<DecoratedObject> numberFormatHandle =
+      getNumberFormat(createPseudoHandle(nf), runtime);
 
   // Since numberFormatHandle came out of an internal slot, it's an
   // assertable failure if it has the wrong type.

lib/VM/JSLib/Proxy.cpp

@@ -103,8 +103,8 @@ proxyConstructor(void *, Runtime *runtime, NativeArgs args) {
 CallResult<HermesValue>
 proxyRevocationSteps(void *, Runtime *runtime, NativeArgs args) {
   // 1. Let p be F.[[RevocableProxy]].
-  auto cc = runtime->getCurrentFrame()->getCalleeClosure();
-  auto revoker = vmcast<NativeFunction>(cc);
+  auto revoker = vmcast<NativeFunction>(
+      runtime->getCurrentFrame()->getCalleeClosureUnsafe());
   HermesValue proxyVal =
       getRevocableProxySlot(revoker, runtime).unboxToHV(runtime);
   // 2. If p is null, return undefined.

lib/VM/Profiler/SamplingProfilerPosix.cpp

@@ -255,7 +255,7 @@ uint32_t SamplingProfiler::walkRuntimeStack(
         registerDomain(module->getDomainForSamplingProfiler());
     } else if (
         auto *nativeFunction =
-            dyn_vmcast_or_null<NativeFunction>(frame.getCalleeClosure())) {
+            dyn_vmcast<NativeFunction>(frame.getCalleeClosureUnsafe())) {
       frameStorage.kind = vmisa<FinalizableNativeFunction>(nativeFunction)
           ? StackFrame::FrameKind::FinalizableNativeFunction
           : StackFrame::FrameKind::NativeFunction;

lib/VM/Runtime.cpp

@@ -1733,7 +1733,7 @@ void Runtime::dumpCallFrames(llvh::raw_ostream &OS) {
   unsigned i = 0;
   for (StackFramePtr sf : getStackFrames()) {
     OS << i++ << " ";
-    if (auto *closure = sf.getCalleeClosure()) {
+    if (auto *closure = dyn_vmcast<Callable>(sf.getCalleeClosureOrCBRef())) {
       OS << cellKindStr(closure->getKind()) << " ";
     }
     if (auto *cb = sf.getCalleeCodeBlock()) {