[8.2.1] Cherry-pick of flag/StarlarkSemantics related changes (#25850)

Original commits:

- 85dd88a
- a5799c3
- f934630

Fixes #25846 for 8.2.1.

---------

Co-authored-by: Googler <ilist@google.com>

Author: Wyverald

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java

@@ -444,8 +444,7 @@ public void registerToolchains(
             named = true,
             positional = false,
             defaultValue = "False",
-            enableOnlyWithFlag = "-experimental_isolated_extension_usages",
-            valueWhenDisabled = "False"),
+            enableOnlyWithFlag = "-experimental_isolated_extension_usages"),
       },
       useStarlarkThread = true)
   public ModuleExtensionProxy useExtension(

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java

@@ -352,7 +352,6 @@ Descriptor stringAttribute(
             name = MATERIALIZER_ARG,
             enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_DORMANT_DEPS,
             allowedTypes = {@ParamType(type = StarlarkFunction.class)},
-            valueWhenDisabled = "None",
             defaultValue = "None",
             named = true,
             positional = false,
@@ -637,7 +636,6 @@ Descriptor intListAttribute(
             name = MATERIALIZER_ARG,
             enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_DORMANT_DEPS,
             allowedTypes = {@ParamType(type = StarlarkFunction.class)},
-            valueWhenDisabled = "None",
             defaultValue = "None",
             named = true,
             positional = false,

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java

@@ -493,7 +493,6 @@ name to an attribute object (see
             named = true,
             positional = false,
             defaultValue = "None",
-            valueWhenDisabled = "None",
             disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM,
             doc =
                 "This parameter has been deprecated. Migrate rules to use"

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcModuleApi.java

@@ -1503,7 +1503,6 @@ LinkerInputT createLinkerInput(
             named = true,
             disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
             defaultValue = "None",
-            valueWhenDisabled = "None",
             allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
         @Param(
             name = "user_link_flags",
@@ -1512,7 +1511,6 @@ LinkerInputT createLinkerInput(
             named = true,
             disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
             defaultValue = "None",
-            valueWhenDisabled = "None",
             allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
         @Param(
             name = "additional_inputs",
@@ -1521,7 +1519,6 @@ LinkerInputT createLinkerInput(
             named = true,
             disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
             defaultValue = "None",
-            valueWhenDisabled = "None",
             allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
         @Param(
             name = "extra_link_time_library",

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java

@@ -90,8 +90,7 @@ default JavaInfoT mergeJavaProviders(Sequence<?> providers /* <JavaInfoT> expect
                 "Deprecated: The output jar of the rule. Used to name the resulting source jar. "
                     + "The parameter sets output_source_jar parameter to `{output_jar}-src.jar`."
                     + "Use output_source_jar parameter directly instead.",
-            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS,
-            valueWhenDisabled = "None"),
+            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS),
         @Param(
             name = "output_source_jar",
             positional = false,
@@ -129,8 +128,7 @@ default JavaInfoT mergeJavaProviders(Sequence<?> providers /* <JavaInfoT> expect
                 "Deprecated: You can drop this parameter (host_javabase is provided with "
                     + "java_toolchain)",
             defaultValue = "None",
-            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS,
-            valueWhenDisabled = "None"),
+            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS),
       })
   default FileApi packSources(
       StarlarkActionFactoryT actions,
@@ -360,8 +358,7 @@ default FileApi runIjar(
                 "Deprecated: You can drop this parameter (host_javabase is provided with "
                     + "java_toolchain)",
             defaultValue = "None",
-            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS,
-            valueWhenDisabled = "None"),
+            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS),
         @Param(
             name = "sourcepath",
             positional = false,

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java

@@ -109,8 +109,7 @@ dependency on a label to a file (a repository rule cannot depend on a generated
             doc = "Compatible with remote execution",
             named = true,
             positional = false,
-            enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC,
-            valueWhenDisabled = "False"),
+            enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC),
         @Param(
             name = "doc",
             allowedTypes = {

src/main/java/net/starlark/java/annot/Param.java

@@ -100,6 +100,9 @@
    *
    * <p>Note that at most one of {@link #enableOnlyWithFlag} and {@link #disableWithFlag} can be
    * non-empty.
+   *
+   * <p>If {@link #enableOnlyWithFlag} is non-empty, then {@link #defaultValue} must also be
+   * non-empty; mandatory parameters cannot be toggled by a flag.
    */
   String enableOnlyWithFlag() default "";
 
@@ -110,27 +113,9 @@
    *
    * <p>Note that at most one of {@link #enableOnlyWithFlag} and {@link #disableWithFlag} can be
    * non-empty.
-   */
-  String disableWithFlag() default "";
-
-  /**
-   * Value for the parameter when the parameter is "disabled" based on semantic flags. (When the
-   * parameter is disabled, it may not be set from Starlark, but an argument of the given value is
-   * passed to the annotated Java method when invoked.) (See {@link #enableOnlyWithFlag()} and
-   * {@link #disableWithFlag()} for toggling a parameter with semantic flags.
    *
-   * <p>The parameter value is written as a Starlark expression (for example: "False", "True", "[]",
-   * "None").
-   *
-   * <p>This should be set (non-empty) if and only if the parameter may be disabled with a semantic
-   * flag.
-   *
-   * <p>Note that this is very similar to {@link #defaultValue}; it may be considered "the default
-   * value if no parameter is specified". It is important that this is distinct, however, in cases
-   * where it is desired to have a normally-mandatory parameter toggled by flag. Such a parameter
-   * should have no {@link #defaultValue} set, but should have a sensible {@link
-   * #valueWhenDisabled()} set. ("unbound" may be used in cases where no value would be valid. See
-   * {@link #defaultValue}.)
+   * <p>If {@link #disableWithFlag} is non-empty, then {@link #defaultValue} must also be non-empty;
+   * mandatory parameters cannot be toggled by a flag.
    */
-  String valueWhenDisabled() default "";
+  String disableWithFlag() default "";
 }

src/main/java/net/starlark/java/annot/processor/StarlarkMethodProcessor.java

@@ -381,13 +381,10 @@ private void checkParameter(Element param, Param paramAnnot) {
       }
       hasFlag = true;
     }
-    if (hasFlag == paramAnnot.valueWhenDisabled().isEmpty()) {
+    if (hasFlag && paramAnnot.defaultValue().isEmpty()) {
       errorf(
           param,
-          hasFlag
-              ? "Parameter '%s' may be disabled by semantic flag, thus valueWhenDisabled must be"
-                  + " set"
-              : "Parameter '%s' has valueWhenDisabled set, but is always enabled",
+          "Parameter '%s' may be disabled by semantic flag, thus defaultValue must be" + " set",
           paramAnnot.name());
     }
 

src/main/java/net/starlark/java/eval/BuiltinFunction.java

@@ -41,27 +41,11 @@ public final class BuiltinFunction implements StarlarkCallable {
 
   private final Object obj;
   private final String methodName;
-  @Nullable private final MethodDescriptor desc;
-
-  /**
-   * Constructs a BuiltinFunction for a StarlarkMethod-annotated method of the given name (as seen
-   * by Starlark, not Java).
-   */
-  BuiltinFunction(Object obj, String methodName) {
-    this.obj = obj;
-    this.methodName = methodName;
-    this.desc = null; // computed later
-  }
+  private final MethodDescriptor desc;
 
   /**
    * Constructs a BuiltinFunction for a StarlarkMethod-annotated method (not field) of the given
    * name (as seen by Starlark, not Java).
-   *
-   * <p>This constructor should be used only for ephemeral BuiltinFunction values created
-   * transiently during a call such as {@code x.f()}, when the caller has already looked up the
-   * MethodDescriptor using the same semantics as the thread that will be used in the call. Use the
-   * other (slower) constructor if there is any possibility that the semantics of the {@code x.f}
-   * operation differ from those of the thread used in the call.
    */
   BuiltinFunction(Object obj, String methodName, MethodDescriptor desc) {
     Preconditions.checkArgument(!desc.isStructField());
@@ -73,28 +57,17 @@ public final class BuiltinFunction implements StarlarkCallable {
   @Override
   public Object fastcall(StarlarkThread thread, Object[] positional, Object[] named)
       throws EvalException, InterruptedException {
-    MethodDescriptor desc = getMethodDescriptor(thread.getSemantics());
+    desc.checkEnabled(thread);
     Object[] vector = getArgumentVector(thread, desc, positional, named);
     return desc.call(
         obj instanceof String ? StringModule.INSTANCE : obj, vector, thread.mutability());
   }
 
-  private MethodDescriptor getMethodDescriptor(StarlarkSemantics semantics) {
-    MethodDescriptor desc = this.desc;
-    if (desc == null) {
-      desc = CallUtils.getAnnotatedMethods(semantics, obj.getClass()).get(methodName);
-      Preconditions.checkArgument(
-          !desc.isStructField(),
-          "BuiltinFunction constructed for MethodDescriptor(structField=True)");
-    }
-    return desc;
-  }
-
   /**
    * Returns the StarlarkMethod annotation of this Starlark-callable Java method.
    */
   public StarlarkMethod getAnnotation() {
-    return getMethodDescriptor(StarlarkSemantics.DEFAULT).getAnnotation();
+    return desc.getAnnotation();
   }
 
   @Override
@@ -126,7 +99,6 @@ public String toString() {
    * StarlarkMethod-annotated Java method.
    *
    * @param thread the Starlark thread for the call
-   * @param loc the location of the call expression, or BUILTIN for calls from Java
    * @param desc descriptor for the StarlarkMethod-annotated method
    * @param positional an array of positional arguments; as an optimization, in simple cases, this
    *     array may be reused as the method's return value
@@ -196,7 +168,7 @@ private Object[] getArgumentVector(
       }
 
       // disabled?
-      if (param.disabledByFlag() != null) {
+      if (!param.isEnabled(thread)) {
         // Skip disabled parameter as if not present at all.
         // The default value will be filled in below.
         continue;
@@ -269,13 +241,12 @@ private Object[] getArgumentVector(
       }
 
       // disabled?
-      String flag = param.disabledByFlag();
-      if (flag != null) {
+      if (!param.isEnabled(thread)) {
         // spill to **kwargs
         if (kwargs == null) {
           throw Starlark.errorf(
               "in call to %s(), parameter '%s' is %s",
-              methodName, param.getName(), disabled(flag, thread.getSemantics()));
+              methodName, param.getName(), param.getDisabledErrorMessage());
         }
 
         // duplicate named argument?
@@ -375,21 +346,4 @@ private void checkParamValue(ParamDescriptor param, Object value) throws EvalExc
           methodName, param.getName(), Starlark.type(value), param.getTypeErrorMessage());
     }
   }
-
-  // Returns a phrase meaning "disabled" appropriate to the specified flag.
-  private static String disabled(String flag, StarlarkSemantics semantics) {
-    // If the flag is True, it must be a deprecation flag. Otherwise it's an experimental flag.
-    // TODO(adonovan): is that assumption sound?
-    if (semantics.getBool(flag)) {
-      return String.format(
-          "deprecated and will be removed soon. It may be temporarily re-enabled by setting"
-              + " --%s=false",
-          flag.substring(1)); // remove [+-] prefix
-    } else {
-      return String.format(
-          "experimental and thus unavailable with the current flags. It may be enabled by setting"
-              + " --%s",
-          flag.substring(1)); // remove [+-] prefix
-    }
-  }
 }

src/main/java/net/starlark/java/eval/CallUtils.java

@@ -148,7 +148,7 @@ private static StarlarkClassDescriptor buildStarlarkClassDescriptor(
         continue;
       }
 
-      MethodDescriptor descriptor = MethodDescriptor.of(method, callable, semantics);
+      MethodDescriptor descriptor = MethodDescriptor.of(method, callable);
 
       // self-call method?
       if (callable.selfCall()) {
@@ -200,7 +200,7 @@ static Object getAnnotatedField(StarlarkSemantics semantics, Object x, String fi
     if (desc == null) {
       throw Starlark.errorf("value of type %s has no .%s field", Starlark.type(x), fieldName);
     }
-    return desc.callField(x, semantics, /*mu=*/ null);
+    return desc.callField(x, semantics, /* mu= */ null);
   }
 
   /** Returns the names of the Starlark fields of {@code x} under the specified semantics. */

src/main/java/net/starlark/java/eval/MethodDescriptor.java

@@ -24,7 +24,9 @@
 import java.util.Arrays;
 import javax.annotation.Nullable;
 import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkAnnotations;
 import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.ParamDescriptor.ConditionalCheck;
 
 /**
  * A value class to store Methods with their corresponding {@link StarlarkMethod} annotation
@@ -35,7 +37,7 @@
  */
 final class MethodDescriptor {
   private final Method method;
-  private final StarlarkMethod annotation;
+  @Nullable private transient StarlarkMethod annotation;
 
   private final String name;
   private final String doc;
@@ -50,6 +52,8 @@ final class MethodDescriptor {
   private final boolean useStarlarkSemantics;
   private final boolean positionalsReusableAsJavaArgsVectorIfArgumentCountValid;
 
+  @Nullable private final ConditionalCheck conditionalCheck;
+
   private enum HowToHandleReturn {
     NULL_TO_NONE, // any Starlark value; null -> None
     ERROR_ON_NULL, // any Starlark value; null -> error
@@ -110,30 +114,40 @@ private MethodDescriptor(
             && !useStarlarkSemantics
             && !useStarlarkThread
             && stream(parameters).allMatch(MethodDescriptor::paramUsableAsPositionalWithoutChecks);
+
+    if (!annotation.enableOnlyWithFlag().isEmpty() || !annotation.disableWithFlag().isEmpty()) {
+      conditionalCheck =
+          new ConditionalCheck(annotation.enableOnlyWithFlag(), annotation.disableWithFlag());
+    } else {
+      conditionalCheck = null;
+    }
   }
 
   private static boolean paramUsableAsPositionalWithoutChecks(ParamDescriptor param) {
     return param.isPositional()
-        && param.disabledByFlag() == null
+        && param.conditionalCheck == null
         && param.getAllowedClasses() == null;
   }
 
   /** Returns the StarlarkMethod annotation corresponding to this method. */
   StarlarkMethod getAnnotation() {
+    if (annotation == null) {
+      // Annotation is null on deserialization, becuase deserializer can't handle annotations
+      annotation = StarlarkAnnotations.getStarlarkMethod(method);
+    }
     return annotation;
   }
 
-  /** @return Starlark method descriptor for provided Java method and signature annotation. */
-  static MethodDescriptor of(
-      Method method, StarlarkMethod annotation, StarlarkSemantics semantics) {
+  /** Returns starlark method descriptor for provided Java method and signature annotation. */
+  static MethodDescriptor of(Method method, StarlarkMethod annotation) {
     // This happens when the interface is public but the implementation classes
     // have reduced visibility.
     method.setAccessible(true);
 
     Class<?>[] paramClasses = method.getParameterTypes();
     Param[] paramAnnots = annotation.parameters();
     ParamDescriptor[] params = new ParamDescriptor[paramAnnots.length];
-    Arrays.setAll(params, i -> ParamDescriptor.of(paramAnnots[i], paramClasses[i], semantics));
+    Arrays.setAll(params, i -> ParamDescriptor.of(paramAnnots[i], paramClasses[i]));
 
     return new MethodDescriptor(
         method,
@@ -317,4 +331,31 @@ boolean isSelfCall() {
   boolean isPositionalsReusableAsJavaArgsVectorIfArgumentCountValid() {
     return positionalsReusableAsJavaArgsVectorIfArgumentCountValid;
   }
+
+  /** Returns true if parameter is enabled. */
+  void checkEnabled(StarlarkThread thread) throws EvalException {
+    if (conditionalCheck == null) { // fast path
+      return;
+    }
+
+    // TODO(b/407506132): A method enabled by a non-experimental flag should not be marked as
+    //  experimental
+    if (!thread
+        .getSemantics()
+        .isFeatureEnabledBasedOnTogglingFlags(
+            conditionalCheck.enableOnlyWithFlag(), conditionalCheck.disableWithFlag())) {
+      if (!conditionalCheck.enableOnlyWithFlag().isEmpty()) {
+        throw Starlark.errorf(
+            "function %s() is experimental and thus unavailable with the current flags. It may be"
+                + " enabled by setting --%s",
+            name, conditionalCheck.enableOnlyWithFlag().substring(1)); // remove [+-] prefix
+      }
+      if (!conditionalCheck.disableWithFlag().isEmpty()) {
+        throw Starlark.errorf(
+            "function %s() is deprecated and will be removed soon. It may be temporarily re-enabled"
+                + " by setting --%s",
+            name, conditionalCheck.disableWithFlag().substring(1)); // remove [+-] prefix
+      }
+    }
+  }
 }