Fix -commonjs compilation crash and empty output (#1990)

lib/IRGen/ESTreeIRGen.cpp

@@ -303,6 +303,13 @@ void ESTreeIRGen::doCJSModule(
   assert(Root && "no root in ESTreeIRGen");
   auto *func = cast<ESTree::FunctionExpressionNode>(Root);
 
+  // Create a top level FunctionContext that will never be executed, because
+  // genBasicFunction reads state from the current function context (e.g. the
+  // class contexts captured when enqueueing the compilation of the module
+  // function).
+  FunctionContext topLevelFunctionContext{
+      this, Mod->getTopLevelFunction(), nullptr};
+
   // Take care of the additions to the global scope that this module could
   // have done. A module can only add ambient global properties. Look for new
   // ones (customData == nullptr) and declare them.

lib/Optimizer/Scalar/FunctionAnalysis.cpp

@@ -406,8 +406,9 @@ void analyzeFunctionCallsites(Function *F) {
   // Attempt to start from a position of knowing all callsites.
   F->getAttributesRef(M)._allCallsitesKnownInStrictMode = true;
 
-  if (F->isGlobalScope()) {
-    // global function is called by the runtime, so its callsites aren't known.
+  if (F->isGlobalScope() || M->findCJSModule(F)) {
+    // The global function and CommonJS module functions are called by the
+    // runtime, so their callsites aren't known.
     F->getAttributesRef(M)._allCallsitesKnownInStrictMode = false;
   }
 

lib/Optimizer/Scalar/Utils.cpp

@@ -243,8 +243,9 @@ bool deleteUnusedFunctionsAndVariables(Module *M) {
     for (auto &F : *M) {
       // Delete any functions that do not have any uses other than in their own
       // bodies. The top level function does not have an explicit user, so check
-      // for it directly.
-      if (&F != M->getTopLevelFunction() &&
+      // for it directly. CommonJS module functions are only referenced by the
+      // module's CJS module table, so they must also be kept.
+      if (&F != M->getTopLevelFunction() && !M->findCJSModule(&F) &&
           llvh::all_of(F.getUsers(), [&F](Instruction *user) {
             // Use must be from another function to be meaningful.
             return user->getFunction() == &F;

test/hermes/cjs-module.js

@@ -0,0 +1,24 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -commonjs %s | %FileCheck --match-full-lines %s
+// RUN: %hermes -O0 -commonjs %s | %FileCheck --match-full-lines %s
+// RUN: %hermes -O -commonjs %s | %FileCheck --match-full-lines %s
+
+// Regression test for #1990: compiling with -commonjs used to crash IRGen
+// because doCJSModule ran without an active FunctionContext, and the
+// optimizer used to delete CJS module functions as unused.
+
+print('cjs module start');
+// CHECK: cjs module start
+
+print(typeof exports, typeof require, typeof module);
+// CHECK-NEXT: object function object
+
+exports.foo = 42;
+print(module.exports.foo);
+// CHECK-NEXT: 42