fix FIŘ tests + pre-commit hook

TODO: record promise forces in interpreter

Author: Jakobeha

.githooks/pre-commit.sh

@@ -3,8 +3,8 @@
 SERVER_DIR="server"
 MVN="mvn -f $SERVER_DIR"
 
-staged_files=$(git diff --staged --name-only -- $SERVER_DIR)
-everything_staged=$(git diff --name-only -- $SERVER_DIR)
+staged_files=$(git diff --staged --name-only --diff-filter=d -- $SERVER_DIR)
+everything_staged=$(git diff --name-only --diff-filter=d -- $SERVER_DIR)
 # `partially_staged` from https://github.com/evilmartians/lefthook/issues/140#issuecomment-1012967850
 partially_staged=$(comm -12 <(echo "$everything_staged" | sort) <(echo "$staged_files" | sort))
 format_only_staged=-DspotlessFiles="$(echo "$staged_files" | sed 's/^/.*/' | paste -sd ',' -)"

client/rsh/src/bc2c/runtime.c

@@ -24,4 +24,4 @@ Rsh_Math1Fun R_MATH1_EXT_FUNS[] = {X_MATH1_EXT_OPS};
 SEXP NOT_OP;
 SEXP LOG_OP;
 
-#include "../common2c/runtime_impl.h"
+#include "runtime_impl.h"

client/rsh/src/fir2c/runtime.c

@@ -1098,7 +1098,7 @@ void Fir_dbg_signature(Fir_Signature signature) {
   fprintf(stderr, "\n");
 }
 
-DEFINE_INTRINSIC(SEXP, checkFun, value_fx_none_ret_value, SEXP value) {
+DEFINE_INTRINSIC(SEXP, checkFun, value_fx_impure_ret_value, SEXP value) {
   if (TYPEOF(value) != CLOSXP && TYPEOF(value) != BUILTINSXP &&
       TYPEOF(value) != SPECIALSXP) {
     Rf_error("attempt to apply non-function");
@@ -1107,7 +1107,7 @@ DEFINE_INTRINSIC(SEXP, checkFun, value_fx_none_ret_value, SEXP value) {
   return R_NilValue;
 }
 
-DEFINE_INTRINSIC(SEXP, checkMissing, value_fx_none_ret_value, SEXP value) {
+DEFINE_INTRINSIC(SEXP, checkMissing, value_fx_impure_ret_value, SEXP value) {
   if (value == R_MissingArg) {
     Rf_error("argument is missing, with no default");
   }

client/rsh/src/fir2c/runtime.h

@@ -188,8 +188,8 @@ void Fir_dbg_signature(Fir_Signature signature);
 
 #pragma clang diagnostic pop
 
-DEFINE_INTRINSIC(SEXP, checkFun, value_fx_none_ret_value, SEXP value);
-DEFINE_INTRINSIC(SEXP, checkMissing, value_fx_none_ret_value, SEXP value);
+DEFINE_INTRINSIC(SEXP, checkFun, value_fx_impure_ret_value, SEXP value);
+DEFINE_INTRINSIC(SEXP, checkMissing, value_fx_impure_ret_value, SEXP value);
 DEFINE_INTRINSIC(SEXP, toForSeq, value_fx_none_ret_value, SEXP value);
 DEFINE_INTRINSIC(SEXP, toForSeq, vec_logical_fx_none_ret_vec_logical, SEXP value);
 DEFINE_INTRINSIC(SEXP, toForSeq, vec_int_fx_none_ret_vec_int, SEXP value);
@@ -412,11 +412,6 @@ DEFINE_OVERRIDDEN_BUILTIN(bool, is_u2echaracter, value_fx_none_ret_bool, SEXP va
 DEFINE_OVERRIDDEN_BUILTIN(bool, is_u2eobject, value_fx_none_ret_bool, SEXP value);
 DEFINE_OVERRIDDEN_BUILTIN(bool, is_u2esymbol, value_fx_none_ret_bool, SEXP value);
 
-// Stubs for BC->C tests
-SEXP Rsh_initialize_runtime(void) { return R_NilValue; }
-SEXP Rsh_pc_get(void) { return R_NilValue; }
-SEXP Rsh_pc_reset(void) { return R_NilValue; }
-
 #ifdef __cplusplus
 }
 #endif

server/src/main/java/org/prlprg/fir/analyze/Analyses.java

@@ -8,6 +8,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -32,7 +33,7 @@ public class Analyses {
   private final AnalysisTypeMap<Analysis> abstractionAnalysisTypes;
   private final AnalysisTypeMap<CfgAnalysis> cfgAnalysisTypes;
 
-  private final Analysis[] abstractionAnalyses;
+  private final @Nullable Analysis[] abstractionAnalyses;
   private final Map<CFG, CfgAnalysis[]> cfgAnalyses = new HashMap<>();
 
   public Analyses(Abstraction target, Class<?>... analysisClasses) {
@@ -48,13 +49,6 @@ public Analyses(Abstraction target, AnalysisTypes analysisTypes) {
     abstractionAnalyses = new Analysis[abstractionAnalysisTypes.size()];
   }
 
-  /// Run all abstraction analyses.
-  public void forceAbstractionAnalyses() {
-    for (var analysisClass : abstractionAnalysisTypes.inherent) {
-      get(analysisClass);
-    }
-  }
-
   /// Clear all analyses so they must be recomputed.
   public void evict() {
     Arrays.fill(abstractionAnalyses, null);
@@ -82,7 +76,7 @@ private <T extends Analysis> T get(Class<T> analysisType, boolean allowDependenc
 
     // Cast is safe because forall `T`,
     // `analysisTypes.get(T.class).analysisConstructor.newInstance(...) instanceof T`.
-    return (T) abstractionAnalyses[info.index];
+    return (T) Objects.requireNonNull(abstractionAnalyses[info.index]);
   }
 
   /// @throws NoSuchElementException If `cfg` isn't in the `Abstraction` provided to the

server/src/main/java/org/prlprg/fir/analyze/resolve/OriginAnalysis.java

@@ -462,12 +462,16 @@ public State copy() {
 
     @Override
     public void merge(State other) {
-      // Merge origins = keep only those that are present in both and equal
-      mergeOrigins(registerOrigins, other.registerOrigins);
-      mergeOrigins(variableOrigins, other.variableOrigins);
+      // Merge register origins = assume they're the same except for `null`s, unify
+      // (because we may get origins of unreachable phis, for the workaround in `run(Jump)`).
+      // Without support for the workaround, it would be the same as merging variable origins
+      registerOrigins.putAll(other.registerOrigins);
+      // Merge variable origins = keep only those that are present in both and equal
+      mergeVariableOrigins(variableOrigins, other.variableOrigins);
     }
 
-    private static <T> void mergeOrigins(Map<T, Argument> origins, Map<T, Argument> otherOrigins) {
+    private static <T> void mergeVariableOrigins(
+        Map<T, Argument> origins, Map<T, Argument> otherOrigins) {
       for (var iterator = origins.entrySet().iterator(); iterator.hasNext(); ) {
         var entry = iterator.next();
         var variable = entry.getKey();

server/src/main/java/org/prlprg/fir/interpret/internal/Builtins.java

@@ -33,7 +33,7 @@
 import org.prlprg.sexp.VecSXP;
 
 /// Java implementations of GNU-R builtins for [InternalInterpreter] (specifically
-/// [InternalInterpreter#registerExternal(String, ExternalVersion)]).
+/// [InternalInterpreter#registerExternal(String, Signature, ExternalVersion)]).
 ///
 /// Note that these are only behaviorally equivalent to GNU-R implementations in some common
 /// scenarios. They're not a complete substitute for GNU-R, but a way to test functions which
@@ -227,7 +227,7 @@ public static void registerBuiltins(InternalInterpreter interpreter) {
         "setVisible", sig(Type.ANY_VALUE_SEXP, Effects.NONE), Builtins::setVisible);
     interpreter.registerExternal(
         "checkMissing",
-        sig(Type.ANY_VALUE_SEXP, Effects.NONE, Type.ANY_VALUE_SEXP),
+        sig(Type.ANY_VALUE_SEXP, Effects.IMPURE, Type.ANY_VALUE_SEXP),
         Builtins::checkMissing);
     interpreter.registerExternal(
         "toForSeq",

server/src/main/java/org/prlprg/fir/opt/AbstractionOptimization.java

@@ -3,6 +3,8 @@
 import static org.prlprg.fir.opt.Cleanup.cleanup;
 
 import java.util.List;
+import java.util.Optional;
+import org.jspecify.annotations.Nullable;
 import org.prlprg.AppConfig;
 import org.prlprg.fir.feedback.AbstractionFeedback;
 import org.prlprg.fir.feedback.ModuleFeedback;
@@ -12,12 +14,13 @@
 /// An optimization that runs on an [Abstraction].
 public interface AbstractionOptimization extends Optimization {
   /// Returns `true` if it made progress.
-  default boolean run(Function function, AbstractionFeedback feedback, Abstraction abstraction) {
-    return function
-        .owner()
+  default boolean run(
+      @Nullable Function function, AbstractionFeedback feedback, Abstraction abstraction) {
+    return abstraction
+        .module()
         .record(
             "AbstractionOptimization#run",
-            List.of(this, function, feedback, abstraction),
+            List.of(this, Optional.ofNullable(function), feedback, abstraction),
             () -> {
               var changed = runWithoutRecording(function, feedback, abstraction);
 
@@ -45,5 +48,5 @@ default boolean runWithoutRecording(ModuleFeedback feedback, Function function)
 
   /// Returns `true` if it made progress.
   boolean runWithoutRecording(
-      Function function, AbstractionFeedback feedback, Abstraction abstraction);
+      @Nullable Function function, AbstractionFeedback feedback, Abstraction abstraction);
 }

server/src/main/java/org/prlprg/fir/opt/Cleanup.java

@@ -72,7 +72,7 @@ public static boolean cleanup(Abstraction abstraction) {
 
   @Override
   public boolean runWithoutRecording(
-      Function function, AbstractionFeedback feedback, Abstraction abstraction) {
+      @Nullable Function function, AbstractionFeedback feedback, Abstraction abstraction) {
     var opt = new OnAbstraction(abstraction);
     opt.run();
     return reportChanges && opt.changed;

server/src/main/java/org/prlprg/fir/opt/CreateBestVersion.java

@@ -3,22 +3,23 @@
 import static org.prlprg.fir.ir.abstraction.AbstractionCopier.copy;
 
 import com.google.common.collect.ImmutableList;
-import org.jspecify.annotations.NonNull;
+import java.util.Objects;
+import org.jspecify.annotations.Nullable;
 import org.prlprg.fir.feedback.AbstractionFeedback;
 import org.prlprg.fir.ir.abstraction.Abstraction;
 import org.prlprg.fir.ir.callee.StaticFnCallee;
 import org.prlprg.fir.ir.expression.Call;
 import org.prlprg.fir.ir.instruction.Statement;
 import org.prlprg.fir.ir.module.Function;
-import org.prlprg.fir.ir.type.Type;
 import org.prlprg.fir.opt.specialize.OptimizeCallee;
+import org.prlprg.util.Lists;
 
 /// If there's a call whose best guaranteed version
 /// ([Function#guess(org.prlprg.fir.ir.type.Signature)]) does not exactly match its arguments'
 /// static types, copy that version into a new one and narrow the parameter types to match
 public record CreateBestVersion(int versionLimit) implements AbstractionOptimization {
   public boolean runWithoutRecording(
-      Function function, AbstractionFeedback feedback, Abstraction version) {
+      @Nullable Function function, AbstractionFeedback feedback, Abstraction version) {
     var calls =
         version
             .streamCfgs()
@@ -44,16 +45,15 @@ private boolean run(AbstractionFeedback feedback, Abstraction scope, Call call)
     }
 
     var callArguments = call.callArguments();
-    var argumentTypes =
-        callArguments.stream().map(scope::typeOf).collect(ImmutableList.toImmutableList());
-    if (argumentTypes.contains(null)) {
+    if (callArguments.stream().anyMatch(a -> scope.typeOf(a) == null)) {
       // Invalid, null type
       return false;
     }
-    @SuppressWarnings("RedundantCast")
-    var argumentTypes1 = (ImmutableList<@NonNull Type>) argumentTypes;
+    var argumentTypes =
+        ImmutableList.copyOf(
+            Lists.mapStrict(callArguments, a -> Objects.requireNonNull(scope.typeOf(a))));
 
-    var bestSignature = OptimizeCallee.bestSignature(callee, argumentTypes1);
+    var bestSignature = OptimizeCallee.bestSignature(callee, argumentTypes);
     var bestVersion = calleeFun.guess(bestSignature);
     if (bestVersion == null) {
       // Invalid, there should always be a possible version
@@ -65,7 +65,7 @@ private boolean run(AbstractionFeedback feedback, Abstraction scope, Call call)
     }
 
     // Create a new version with the exact parameter types
-    copy(feedback.module(), calleeFun, bestVersion, argumentTypes1);
+    copy(feedback.module(), calleeFun, bestVersion, argumentTypes);
     return true;
   }
 }