Flip symbolic macro flags

This enables `--experimental_enable_first_class_macros` and `--incompatible_simplify_unconditional_selects_in_rule_attrs` to true. The former releases Symbolic Macros, and with it, Finalizers and Macro-Aware Visibility. The latter is a quality-of-life improvement for macro users that is technically a breaking change: Any rule instantiated with an attribute value of `select({"//conditions:default": foo})` instead sees just `foo`.

Main milestone of #19922.

RELNOTES: Symbolic Macros -- and with them, Finalizers and the new Macro-Aware Visibility model -- are now generally available (`--experimental_enable_first_class_macros` now defaults to true). Trivial `select()` values are automatically unwrapped (`--incompatible_simplify_unconditional_selects_in_rule_attrs` now defaults to true).
PiperOrigin-RevId: 680779247
Change-Id: Icdbf4b47bc893ba61417eddc22c79d85b6f5d920

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -235,10 +235,10 @@ public final class BuildLanguageOptions extends OptionsBase {
 
   @Option(
       name = "experimental_enable_first_class_macros",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
-      help = "If set to true, enables the `macro()` construct for defining first-class macros.")
+      help = "If set to true, enables the `macro()` construct for defining symbolic macros.")
   public boolean experimentalEnableFirstClassMacros;
 
   @Option(
@@ -787,7 +787,7 @@ public final class BuildLanguageOptions extends OptionsBase {
 
   @Option(
       name = "incompatible_simplify_unconditional_selects_in_rule_attrs",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
       metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
@@ -940,7 +940,7 @@ public StarlarkSemantics toStarlarkSemantics() {
   public static final String EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING =
       "-experimental_single_package_toolchain_binding";
   public static final String EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS =
-      "-experimental_enable_first_class_macros";
+      "+experimental_enable_first_class_macros";
   public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "+experimental_enable_scl_dialect";
   public static final String ENABLE_BZLMOD = "+enable_bzlmod";
   public static final String ENABLE_WORKSPACE = "-enable_workspace";
@@ -1012,7 +1012,7 @@ public StarlarkSemantics toStarlarkSemantics() {
   public static final String INCOMPATIBLE_DISALLOW_CTX_RESOLVE_TOOLS =
       "+incompatible_disallow_ctx_resolve_tools";
   public static final String INCOMPATIBLE_SIMPLIFY_UNCONDITIONAL_SELECTS_IN_RULE_ATTRS =
-      "-incompatible_simplify_unconditional_selects_in_rule_attrs";
+      "+incompatible_simplify_unconditional_selects_in_rule_attrs";
   // non-booleans
   public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
       new StarlarkSemantics.Key<>("experimental_builtins_bzl_path", "%bundled%");

src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java

@@ -46,10 +46,22 @@ public final void createMapper() throws Exception {
         scratchConfiguredTargetAndData(
             "foo",
             "myrule",
-            "cc_binary(",
-            "    name = 'myrule',",
-            "    srcs = [':a.cc'],",
-            "    linkstatic = select({'//conditions:default': 1}))");
+            """
+            # Needed to avoid select() being eliminated as trivial.
+            config_setting(
+                name = "config",
+                values = {"define": "pi=3"},
+            )
+
+            cc_binary(
+                name = "myrule",
+                srcs = [":a.cc"],
+                linkstatic = select({
+                    ":config": 1,
+                    "//conditions:default": 1,
+                }),
+            )
+            """);
 
     RuleConfiguredTarget ct = (RuleConfiguredTarget) ctad.getConfiguredTarget();
     rule = (Rule) ctad.getTargetForTesting();

src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java

@@ -2146,9 +2146,8 @@ public void stringListDictTypeConcatConfigurable() throws Exception {
   }
 
   @Test
-  public void incompatibleSimplifyUnconditionalSelectsInRuleAttrs_checksForNonconfigurableAttrs()
+  public void assigningSelectToNonconfigurableAttr_fails_evenIfSelectIsSimplifiableUnconditional()
       throws Exception {
-    setBuildLanguageOptions("--incompatible_simplify_unconditional_selects_in_rule_attrs=true");
     writeConfigRules();
     scratch.file(
         "foo/BUILD",

src/test/java/com/google/devtools/build/lib/analysis/MacroVisibilityTest.java

@@ -20,7 +20,6 @@
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.testing.junit.testparameterinjector.TestParameterInjector;
 import com.google.testing.junit.testparameterinjector.TestParameters;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -34,11 +33,6 @@
 @RunWith(TestParameterInjector.class)
 public final class MacroVisibilityTest extends BuildViewTestCase {
 
-  @Before
-  public void setUp() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
-  }
-
   @Override
   protected String getDefaultVisibility() {
     // We're testing visibility. Avoid having to litter our test cases with `visibility=` attribute

src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java

@@ -35,21 +35,13 @@
 import java.util.List;
 import javax.annotation.Nullable;
 import org.junit.Assert;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
 /** Tests the execution of symbolic macro implementations. */
 @RunWith(TestParameterInjector.class)
 public final class SymbolicMacroTest extends BuildViewTestCase {
 
-  @Before
-  public void setUp() throws Exception {
-    setBuildLanguageOptions(
-        "--experimental_enable_first_class_macros",
-        "--incompatible_simplify_unconditional_selects_in_rule_attrs");
-  }
-
   /**
    * Returns a package by the given name (no leading "//"), or null upon {@link
    * NoSuchPackageException}.

src/test/java/com/google/devtools/build/lib/analysis/VisibilityProviderTest.java

@@ -32,7 +32,6 @@ public final class VisibilityProviderTest extends BuildViewTestCase {
   @Before
   public void setUp() throws Exception {
     setBuildLanguageOptions(
-        "--experimental_enable_first_class_macros",
         // Let's test the case where input files have proper visibilities by default.
         "--incompatible_no_implicit_file_export");
 

src/test/java/com/google/devtools/build/lib/analysis/select/ConfiguredAttributeMapperTest.java

@@ -306,7 +306,20 @@ public void testNoneValueOnDefaultConditionWithNullDefault() throws Exception {
 
   @Test
   public void testNoneValueOnMandatoryAttribute() throws Exception {
-    scratch.file("a/BUILD", "alias(name='a', actual=select({'//conditions:default': None}))");
+    scratch.file(
+        "a/BUILD",
+        """
+        # Needed to avoid select() being eliminated as trivial.
+        config_setting(
+            name = "config",
+            values = {"define": "pi=3"},
+        )
+
+        alias(
+            name = "a",
+            actual = select({":config": None, "//conditions:default": None}),
+        )
+        """);
     reporter.removeHandler(failFastHandler);
     getConfiguredTarget("//a:a");
     assertContainsEvent("Mandatory attribute 'actual' resolved to 'None'");

src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java

@@ -123,9 +123,18 @@ def _impl(ctx):
         """
         load("//test/starlark:rulestr.bzl", "my_rule", "save_dep")
 
+        # Needed to avoid select() being eliminated as trivial.
+        config_setting(
+            name = "config",
+            values = {"define": "pi=3"},
+        )
+
         my_rule(
             name = "x",
-            dep = select({"//conditions:default": None}),
+            dep = select({
+                ":config": None,
+                "//conditions:default": None,
+            }),
         )
 
         save_dep("x")
@@ -135,7 +144,8 @@ def _impl(ctx):
     assertThat(getConfiguredTarget("//test/getrule:x")).isNotNull();
 
     // We have to compare by stringification because SelectorValue has reference equality semantics.
-    assertThat(getSaved("dep").toString()).isEqualTo("select({\"//conditions:default\": None})");
+    assertThat(getSaved("dep").toString())
+        .isEqualTo("select({\":config\": None, \"//conditions:default\": None})");
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java

@@ -1181,7 +1181,6 @@ public void testPackageDefaultRestrictionDuplicates() throws Exception {
    * <p>The macro does not define any targets.
    */
   private void defineEmptyMacroBzl() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     scratch.file(
         "pkg/my_macro.bzl",
         """
@@ -1372,7 +1371,6 @@ public void testSymbolicMacro_macroPreventsImplicitCreationOfInputFilesUnderItsN
   @Test
   public void testSymbolicMacro_implicitCreationOfInputFilesIsNotTriggeredByMacros()
       throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     scratch.file(
         "pkg/my_macro.bzl",
         """
@@ -1401,7 +1399,6 @@ def _impl(name, visibility):
 
   @Test
   public void testSymbolicMacro_deferredEvaluationExpandsTransitively() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     scratch.file(
         "pkg/my_macro.bzl",
         """
@@ -1456,7 +1453,6 @@ def _impl(name, visibility, height):
 
   @Test
   public void testSymbolicMacro_recursionProhibitedWithEagerEvaluation() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     defineRecursiveMacro(/* deferredEvaluation= */ false);
     expectEvalError(
         """
@@ -1475,7 +1471,6 @@ public void testSymbolicMacro_recursionProhibitedWithEagerEvaluation() throws Ex
 
   @Test
   public void testSymbolicMacro_recursionProhibitedWithDeferredEvaluation() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     defineRecursiveMacro(/* deferredEvaluation= */ true);
     expectEvalError(
         """
@@ -1494,7 +1489,6 @@ public void testSymbolicMacro_recursionProhibitedWithDeferredEvaluation() throws
 
   @Test
   public void testSymbolicMacro_indirectRecursionAlsoProhibited() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     // Define a pair of macros where A calls B calls A (and then would stop, if allowed to get that
     // far). Wrap it in a different entry point to test that the non-cyclic part is included in the
     // traceback.
@@ -1561,7 +1555,6 @@ private void assertVisibilityIs(Target target, String... visibilityLabels) {
   }
 
   private void enableMacrosAndUsePrivateVisibility() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
     // BuildViewTestCase makes everything public by default.
     setPackageOptions("--default_visibility=private");
   }

src/test/java/com/google/devtools/build/lib/packages/RuleFinalizerTest.java

@@ -20,7 +20,6 @@
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import javax.annotation.Nullable;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -29,11 +28,6 @@
 @RunWith(JUnit4.class)
 public final class RuleFinalizerTest extends BuildViewTestCase {
 
-  @Before
-  public void setUp() throws Exception {
-    setBuildLanguageOptions("--experimental_enable_first_class_macros");
-  }
-
   /**
    * Returns a package by the given name (no leading "//"), or null upon {@link
    * NoSuchPackageException}.

src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java

@@ -1678,8 +1678,21 @@ def _rule_impl(ctx):
     scratch.file(
         "test/BUILD",
         """
-        load('//test:aspect.bzl', 'my_rule')
-        my_rule(name = 'xxx', my_attr = select({'//conditions:default': 'foo'}))
+        load("//test:aspect.bzl", "my_rule")
+
+        # Needed to avoid the select() being eliminated as trivial.
+        config_setting(
+            name = "config",
+            values = {"defines": "something"},
+        )
+
+        my_rule(
+            name = "xxx",
+            my_attr = select({
+                ":config": "foo",
+                "//conditions:default": "bar",
+            }),
+        )
         """);
 
     reporter.removeHandler(failFastHandler);

src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java

@@ -660,9 +660,18 @@ public void flagIsAlias_usesSelect() throws Exception {
     scratch.file(
         "test/pkg/BUILD",
         """
+        # Needed to avoid select() being eliminated as trivial.
+        config_setting(
+            name = "config",
+            values = {"define": "pi=3"},
+        )
+
         alias(
             name = "two",
-            actual = select({"//conditions:default": "//test:three"}),
+            actual = select({
+                ":config": "//test:three",
+                "//conditions:default": "//test:three",
+            }),
         )
         """);