bazel syntax: define Starlark.call and use it everywhere

All calls of Starlark callable values go through this function,
whether from the interpreter or Java code. A follow-up change
will make it responsible for doing the push/pop of the call stack.

Also, rename StarlarkCallable.call to callImpl, since no-one should use
it directly. (And reorder its parameters.)

Also, remove sole external call to BaseFunction.callWithArgArray,
and inline it into sole remaining internal caller.

MethodDescriptor.callField now handles the two places that call
a @SkylarkCallable(structField)-annotated Java method.
Previously they were inconsistent in which "extra parameters" they
permitted. Now they allow Location and StarlarkSemantics,
but not StarlarkThread or FuncallExpression.
The annotation processor rejects useStarlarkThreadfor a structField.
(One instance was downgraded to useStarlarkSemantics.)

This is a breaking API change for copybara.

PiperOrigin-RevId: 285031076

src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.Sequence;
+import com.google.devtools.build.lib.syntax.Starlark;
 import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.syntax.StarlarkThread;
 import java.util.List;
@@ -282,7 +283,7 @@ private Object evalFunction(BaseFunction function, ImmutableList<Object> args)
                 .setEventHandler(getEventHandler())
                 .build();
         starlarkContext.storeInThread(thread);
-        return function.call(args, ImmutableMap.of(), null, thread);
+        return Starlark.call(thread, function, /*call=*/ null, args, /*kwargs=*/ ImmutableMap.of());
       }
     }
 

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java

@@ -796,7 +796,7 @@ private static Object applyMapFn(
               .setSemantics(starlarkSemantics)
               .setEventHandler(NullEventHandler.INSTANCE)
               .build();
-      return mapFn.call(args, ImmutableMap.of(), null, thread);
+      return Starlark.call(thread, mapFn, /*call=*/ null, args, /*kwargs=*/ ImmutableMap.of());
     } catch (EvalException e) {
       throw new CommandLineExpansionException(errorMessage(e.getMessage(), location, e.getCause()));
     } catch (InterruptedException e) {
@@ -820,11 +820,15 @@ private static void applyMapEach(
               // TODO(b/77140311): Error if we issue print statements
               .setEventHandler(NullEventHandler.INSTANCE)
               .build();
-      Object[] args = new Object[1];
       int count = originalValues.size();
       for (int i = 0; i < count; ++i) {
-        args[0] = originalValues.get(i);
-        Object ret = mapFn.callWithArgArray(args, null, thread, location);
+        Object ret =
+            Starlark.call(
+                thread,
+                mapFn,
+                /*call=*/ null,
+                originalValues.subList(i, i + 1),
+                /*kwargs=*/ ImmutableMap.of());
         if (ret instanceof String) {
           consumer.accept((String) ret);
         } else if (ret instanceof Sequence) {

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java

@@ -131,11 +131,12 @@ public static ConfiguredTarget buildRule(
       }
 
       Object target =
-          ruleImplementation.call(
+          Starlark.call(
+              thread,
+              ruleImplementation,
+              /*call=*/ null,
               /*args=*/ ImmutableList.of(skylarkRuleContext),
-              /*kwargs*/ ImmutableMap.of(),
-              /*ast=*/ null,
-              thread);
+              /*kwargs=*/ ImmutableMap.of());
 
       if (ruleContext.hasErrors()) {
         return null;

src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java

@@ -38,6 +38,7 @@
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.Starlark;
 import com.google.devtools.build.lib.syntax.StarlarkFunction;
 import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.syntax.StarlarkThread;
@@ -178,15 +179,16 @@ public RepositoryDirectoryValue.Builder fetch(
       // Also we do a lot of stuff in there, maybe blocking operations and we should certainly make
       // it possible to return null and not block but it doesn't seem to be easy with Skylark
       // structure as it is.
-      Object retValue =
-          function.call(
+      Object result =
+          Starlark.call(
+              thread,
+              function,
+              /*call=*/ null,
               /*args=*/ ImmutableList.of(skylarkRepositoryContext),
-              /*kwargs=*/ ImmutableMap.of(),
-              null,
-              thread);
+              /*kwargs=*/ ImmutableMap.of());
       RepositoryResolvedEvent resolved =
           new RepositoryResolvedEvent(
-              rule, skylarkRepositoryContext.getAttr(), outputDirectory, retValue);
+              rule, skylarkRepositoryContext.getAttr(), outputDirectory, result);
       if (resolved.isNewInformationReturned()) {
         env.getListener().handle(Event.debug(resolved.getMessage()));
         env.getListener().handle(Event.debug(rule.getDefinitionInformation()));

src/main/java/com/google/devtools/build/lib/packages/StarlarkCallbackHelper.java

@@ -14,12 +14,14 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.Starlark;
 import com.google.devtools.build.lib.syntax.StarlarkFunction;
 import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.syntax.StarlarkThread;
@@ -77,8 +79,9 @@ public Object call(EventHandler eventHandler, ClassObject ctx, Object... argumen
               .setEventHandler(eventHandler)
               .build();
       context.storeInThread(thread);
-      return callback.call(buildArgumentList(ctx, arguments), null, ast, thread);
-    } catch (ClassCastException | IllegalArgumentException e) {
+      return Starlark.call(
+          thread, callback, ast, buildArgumentList(ctx, arguments), /*kwargs=*/ ImmutableMap.of());
+    } catch (ClassCastException | IllegalArgumentException e) { // TODO(adonovan): investigate
       throw new EvalException(ast.getLocation(), e.getMessage());
     }
   }

src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java

@@ -35,7 +35,7 @@
 import com.google.devtools.build.lib.syntax.Sequence;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import com.google.devtools.build.lib.syntax.StarlarkList;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.util.Fingerprint;
 import java.util.Arrays;
 import java.util.Collection;
@@ -205,7 +205,7 @@ public List<LibraryToLink> getLibraries() {
     }
 
     @Override
-    public Sequence<LibraryToLink> getSkylarkLibrariesToLink(StarlarkThread thread) {
+    public Sequence<LibraryToLink> getSkylarkLibrariesToLink(StarlarkSemantics semantics) {
       return StarlarkList.immutableCopyOf(getLibraries());
     }
 
@@ -404,9 +404,9 @@ public Sequence<String> getSkylarkUserLinkFlags() {
   }
 
   @Override
-  public Object getSkylarkLibrariesToLink(StarlarkThread thread) {
+  public Object getSkylarkLibrariesToLink(StarlarkSemantics semantics) {
     // TODO(plf): Flag can be removed already.
-    if (thread.getSemantics().incompatibleDepsetForLibrariesToLinkGetter()) {
+    if (semantics.incompatibleDepsetForLibrariesToLinkGetter()) {
       return Depset.of(LibraryToLink.TYPE, getLibraries());
     } else {
       return StarlarkList.immutableCopyOf(getLibraries().toList());

src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java

@@ -38,6 +38,7 @@
 import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.syntax.Starlark;
 import com.google.devtools.build.lib.syntax.StarlarkThread;
 import com.google.devtools.build.lib.syntax.StarlarkValue;
 import java.util.Map;
@@ -87,16 +88,14 @@ public ConfiguredAspect create(
               ruleContext.getLabel())
           .storeInThread(thread);
 
-      Object aspectSkylarkObject;
       try {
-        aspectSkylarkObject =
-            skylarkAspect
-                .getImplementation()
-                .call(
-                    /*args=*/ ImmutableList.of(ctadBase.getConfiguredTarget(), skylarkRuleContext),
-                    /* kwargs= */ ImmutableMap.of(),
-                    /*ast=*/ null,
-                    thread);
+        Object aspectSkylarkObject =
+            Starlark.call(
+                thread,
+                skylarkAspect.getImplementation(),
+                /*call=*/ null,
+                /*args=*/ ImmutableList.of(ctadBase.getConfiguredTarget(), skylarkRuleContext),
+                /*kwargs=*/ ImmutableMap.of());
 
         // If allowing analysis failures, targets should be created somewhat normally, and errors
         // will be propagated via a hook elsewhere as AnalysisFailureInfo.

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcLinkingContextApi.java

@@ -20,8 +20,8 @@
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.Depset;
 import com.google.devtools.build.lib.syntax.Sequence;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
 import com.google.devtools.build.lib.syntax.StarlarkValue;
 
 /** Wrapper for every C++ linking provider. */
@@ -44,8 +44,8 @@ public interface CcLinkingContextApi<FileT extends FileApi> extends StarlarkValu
           "Returns the depset of <code>LibraryToLink</code>. May return a list but this is"
               + "deprecated. See #8118.",
       structField = true,
-      useStarlarkThread = true)
-  Object getSkylarkLibrariesToLink(StarlarkThread thread);
+      useStarlarkSemantics = true)
+  Object getSkylarkLibrariesToLink(StarlarkSemantics semantics);
 
   @SkylarkCallable(
       name = "additional_inputs",

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/LinkerInputApi.java

@@ -22,8 +22,8 @@
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Sequence;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
 import com.google.devtools.build.lib.syntax.StarlarkValue;
 
 /** Either libraries, flags or other files that may be passed to the linker as inputs. */
@@ -56,8 +56,8 @@ public interface LinkerInputApi<
               + "deprecated. See #8118.",
       structField = true,
       enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
-      useStarlarkThread = true)
-  Sequence<LibraryToLinkT> getSkylarkLibrariesToLink(StarlarkThread thread);
+      useStarlarkSemantics = true)
+  Sequence<LibraryToLinkT> getSkylarkLibrariesToLink(StarlarkSemantics semantics);
 
   @SkylarkCallable(
       name = "additional_inputs",

src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java

@@ -289,12 +289,20 @@ private void verifyNotStructFieldWithParams(
       throws SkylarkCallableProcessorException {
     if (annotation.structField()) {
       if (annotation.useAst()
+          || annotation.useStarlarkThread()
           || !annotation.extraPositionals().name().isEmpty()
           || !annotation.extraKeywords().name().isEmpty()) {
+        // TODO(adonovan): decide on the restrictions.
+        // - useLocation is needed only by repository_ctx.os. Abolish?
+        // - useStarlarkSemantics is needed only by getSkylarkLibrariesToLink.
+        // - banning useStarlarkThread has not been a problem so far,
+        //   and avoids many tricky problems (especially in StructImpl.equal),
+        //   but it forces implementations to assume Mutability=null,
+        //   which is not quite right.
         throw new SkylarkCallableProcessorException(
             methodElement,
             "@SkylarkCallable-annotated methods with structField=true may not also specify "
-                + "useAst, extraPositionals, or extraKeywords");
+                + "useAst, useStarlarkThread, extraPositionals, or extraKeywords");
       }
     }
   }

src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java

@@ -267,28 +267,29 @@ public Object[] processArguments(
     return arguments;
   }
 
-  /**
-   * The outer calling convention to a BaseFunction.
-   *
-   * @param args a list of all positional arguments (as in *args)
-   * @param kwargs a map for key arguments (as in **kwargs)
-   * @param ast the expression for this function's definition
-   * @param thread the StarlarkThread in the function is called
-   * @return the value resulting from evaluating the function with the given arguments
-   * @throws EvalException-s containing source information.
-   */
   @Override
-  public Object call(
+  public Object callImpl(
+      StarlarkThread thread,
+      @Nullable FuncallExpression call,
       List<Object> args,
-      @Nullable Map<String, Object> kwargs,
-      @Nullable FuncallExpression ast,
-      StarlarkThread thread)
+      Map<String, Object> kwargs)
       throws EvalException, InterruptedException {
-    // ast is null when called from Java (as there's no Skylark call site).
-    Location loc = ast == null ? Location.BUILTIN : ast.getLocation();
+    // call is null when called from Java (as there's no Skylark call site).
+    Location loc = call == null ? Location.BUILTIN : call.getLocation();
 
     Object[] arguments = processArguments(args, kwargs, loc, thread);
-    return callWithArgArray(arguments, ast, thread, getLocation());
+
+    // TODO(adonovan): move Callstack.push/pop into StarlarkThread.push/pop.
+    try {
+      if (Callstack.enabled) {
+        Callstack.push(this);
+      }
+      return call(arguments, call, thread);
+    } finally {
+      if (Callstack.enabled) {
+        Callstack.pop();
+      }
+    }
   }
 
   /**
@@ -307,31 +308,6 @@ protected Object call(Object[] args, @Nullable FuncallExpression ast, StarlarkTh
         String.format("function %s not implemented", getName()));
   }
 
-  /**
-   * The outer calling convention to a BaseFunction. This function expects all arguments to have
-   * been resolved into positional ones.
-   *
-   * @param ast the expression for this function's definition
-   * @param thread the StarlarkThread in the function is called
-   * @return the value resulting from evaluating the function with the given arguments
-   * @throws EvalException-s containing source information.
-   */
-  // TODO(adonovan): make this private. The sole external caller has a location but no ast.
-  public Object callWithArgArray(
-      Object[] arguments, @Nullable FuncallExpression ast, StarlarkThread thread, Location loc)
-      throws EvalException, InterruptedException {
-    try {
-      if (Callstack.enabled) {
-        Callstack.push(this);
-      }
-      return call(arguments, ast, thread);
-    } finally {
-      if (Callstack.enabled) {
-        Callstack.pop();
-      }
-    }
-  }
-
   /**
    * Render this object in the form of an equivalent Python function signature.
    */

src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java

@@ -58,11 +58,8 @@ public final class BuiltinCallable implements StarlarkCallable {
   }
 
   @Override
-  public Object call(
-      List<Object> args,
-      @Nullable Map<String, Object> kwargs,
-      FuncallExpression ast,
-      StarlarkThread thread)
+  public Object callImpl(
+      StarlarkThread thread, FuncallExpression call, List<Object> args, Map<String, Object> kwargs)
       throws EvalException, InterruptedException {
     MethodDescriptor methodDescriptor =
         desc != null ? desc : getMethodDescriptor(thread.getSemantics());
@@ -83,8 +80,9 @@ public Object call(
         Profiler.instance().profile(ProfilerTask.STARLARK_BUILTIN_FN, methodName)) {
       Object[] javaArguments =
           CallUtils.convertStarlarkArgumentsToJavaMethodArguments(
-              thread, ast, methodDescriptor, clazz, args, kwargs);
-      return methodDescriptor.call(objValue, javaArguments, ast.getLocation(), thread);
+              thread, call, methodDescriptor, clazz, args, kwargs);
+      return methodDescriptor.call(
+          objValue, javaArguments, call.getLocation(), thread.mutability());
     }
   }