Skip to content

Commit

Permalink
C++: Add incompatible flag to deprecate old C++ API.
Browse files Browse the repository at this point in the history
Libraries to link have been wrapped by the class LinkerInput for a while, however, as we were not sure we would stay with this refactoring, the Starlark API was guarded by the experimental_cc_shared_library.

This refactoring was needed to be able to implement cc_shared_library. Now that we are sure of the design, we are deprecating the old Starlark API and replacing it with the new one using an incompatible flag.

Incompatible issue: #10860

RELNOTES:none
PiperOrigin-RevId: 297109588
  • Loading branch information
oquenchil authored and Copybara-Service committed Feb 25, 2020
1 parent eb2c107 commit 8efc50e
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,21 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "cc_shared_library will be available")
public boolean experimentalCcSharedLibrary;

@Option(
name = "incompatible_require_linker_input_cc_api",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS, OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, rule create_linking_context will require linker_inputs instead of "
+ "libraries_to_link. The old getters of linking_context will also be disabled and "
+ "just linker_inputs will be available.")
public boolean incompatibleRequireLinkerInputCcApi;

@Option(
name = "experimental_repo_remote_exec",
defaultValue = "false",
Expand Down Expand Up @@ -697,6 +712,7 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleDoNotSplitLinkingCmdline(incompatibleDoNotSplitLinkingCmdline)
.incompatibleUseCcConfigureFromRulesCc(incompatibleUseCcConfigureFromRulesCc)
.incompatibleDepsetForLibrariesToLinkGetter(incompatibleDepsetForLibrariesToLinkGetter)
.incompatibleRequireLinkerInputCcApi(incompatibleRequireLinkerInputCcApi)
.incompatibleRestrictStringEscapes(incompatibleRestrictStringEscapes)
.build();
return INTERNER.intern(semantics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,13 @@ public CcLinkingContext.LinkerInput createLinkerInput(
.build();
}

@Override
public void checkExperimentalCcSharedLibrary(StarlarkThread thread) throws EvalException {
if (!thread.getSemantics().experimentalCcSharedLibrary()) {
throw Starlark.errorf("Pass --experimental_cc_shared_library to use cc_shared_library");
}
}

@Override
public CcLinkingContext createCcLinkingInfo(
Object linkerInputs,
Expand All @@ -681,6 +688,9 @@ public CcLinkingContext createCcLinkingInfo(
StarlarkThread thread)
throws EvalException {
if (EvalUtils.isNullOrNone(linkerInputs)) {
if (thread.getSemantics().incompatibleRequireLinkerInputCcApi()) {
throw Starlark.errorf("linker_inputs cannot be None");
}
@SuppressWarnings("unchecked")
Sequence<LibraryToLink> librariesToLink = nullIfNone(librariesToLinkObject, Sequence.class);
@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public interface CcLinkingContextApi<FileT extends FileApi> extends StarlarkValu
@SkylarkCallable(
name = "user_link_flags",
doc = "Returns the list of user link flags passed as strings.",
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
structField = true)
Sequence<String> getSkylarkUserLinkFlags();

Expand All @@ -43,20 +44,21 @@ public interface CcLinkingContextApi<FileT extends FileApi> extends StarlarkValu
doc =
"Returns the depset of <code>LibraryToLink</code>. May return a list but this is"
+ "deprecated. See #8118.",
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
structField = true,
useStarlarkSemantics = true)
Object getSkylarkLibrariesToLink(StarlarkSemantics semantics);

@SkylarkCallable(
name = "additional_inputs",
doc = "Returns the depset of additional inputs, e.g.: linker scripts.",
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
structField = true)
Depset getSkylarkNonCodeInputs();

@SkylarkCallable(
name = "linker_inputs",
doc = "Returns the depset of linker inputs.",
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
structField = true)
Depset getSkylarkLinkerInputs();
}
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,6 @@ LibraryToLinkT createLibraryLinkerInput(
name = "create_linker_input",
doc = "Creates a <code>LinkingContext</code>.",
useStarlarkThread = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
parameters = {
@Param(
name = "owner",
Expand Down Expand Up @@ -644,6 +643,13 @@ LinkerInputT createLinkerInput(
StarlarkThread thread)
throws EvalException, InterruptedException;

@SkylarkCallable(
name = "check_experimental_cc_shared_library",
doc = "DO NOT USE. This is to guard use of cc_shared_library.",
useStarlarkThread = true,
documented = false)
void checkExperimentalCcSharedLibrary(StarlarkThread thread) throws EvalException;

@SkylarkCallable(
name = "create_linking_context",
doc = "Creates a <code>LinkingContext</code>.",
Expand All @@ -654,33 +660,38 @@ LinkerInputT createLinkerInput(
doc = "Depset of <code>LinkerInput</code>.",
positional = false,
named = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
noneable = true,
valueWhenDisabled = "None",
defaultValue = "None",
allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Depset.class)}),
@Param(
name = "libraries_to_link",
doc = "List of <code>LibraryToLink</code>.",
positional = false,
named = true,
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
noneable = true,
defaultValue = "None",
valueWhenDisabled = "None",
allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
@Param(
name = "user_link_flags",
doc = "List of user link flags passed as strings.",
positional = false,
named = true,
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
noneable = true,
defaultValue = "None",
valueWhenDisabled = "None",
allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
@Param(
name = "additional_inputs",
doc = "For additional inputs to the linking action, e.g.: linking scripts.",
positional = false,
named = true,
disableWithFlag = FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API,
noneable = true,
defaultValue = "None",
valueWhenDisabled = "None",
allowedTypes = {@ParamType(type = NoneType.class), @ParamType(type = Sequence.class)}),
})
LinkingContextT createCcLinkingInfo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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.StarlarkValue;

/** Either libraries, flags or other files that may be passed to the linker as inputs. */
Expand All @@ -36,14 +35,12 @@ public interface LinkerInputApi<
@SkylarkCallable(
name = "owner",
doc = "Returns the owner of this LinkerInput.",
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
structField = true)
Label getSkylarkOwner() throws EvalException;

@SkylarkCallable(
name = "user_link_flags",
doc = "Returns the list of user link flags passed as strings.",
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
structField = true)
Sequence<String> getSkylarkUserLinkFlags();

Expand All @@ -53,14 +50,12 @@ public interface LinkerInputApi<
"Returns the depset of <code>LibraryToLink</code>. May return a list but this is "
+ "deprecated. See #8118.",
structField = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
useStarlarkSemantics = true)
Sequence<LibraryToLinkT> getSkylarkLibrariesToLink(StarlarkSemantics semantics);

@SkylarkCallable(
name = "additional_inputs",
doc = "Returns the depset of additional inputs, e.g.: linker scripts.",
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY,
structField = true)
Sequence<FileT> getSkylarkNonCodeInputs();
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ private FlagIdentifier() {} // uninstantiable
"experimental_starlark_config_transition";
public static final String EXPERIMENTAL_STARLARK_UNUSED_INPUTS_LIST =
"experimental_starlark_unused_inputs_list";
public static final String EXPERIMENTAL_CC_SHARED_LIBRARY = "experimental_cc_shared_library";
public static final String EXPERIMENTAL_REPO_REMOTE_EXEC = "experimental_repo_remote_exec";
public static final String INCOMPATIBLE_APPLICABLE_LICENSES =
"incompatible_applicable_licenses";
Expand All @@ -86,6 +85,8 @@ private FlagIdentifier() {} // uninstantiable
"incompatible_allow_tags_propagation";
public static final String INCOMPATIBLE_REMOVE_ENABLE_TOOLCHAIN_TYPES =
"incompatible_remove_enable_toolchain_types";
public static final String INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API =
"incompatible_require_linker_input_cc_api";
}

// TODO(adonovan): replace the fields of StarlarkSemantics
Expand Down Expand Up @@ -118,8 +119,6 @@ boolean flagValue(String flag) {
return experimentalStarlarkConfigTransitions();
case FlagIdentifier.EXPERIMENTAL_STARLARK_UNUSED_INPUTS_LIST:
return experimentalStarlarkUnusedInputsList();
case FlagIdentifier.EXPERIMENTAL_CC_SHARED_LIBRARY:
return experimentalCcSharedLibrary();
case FlagIdentifier.EXPERIMENTAL_REPO_REMOTE_EXEC:
return experimentalRepoRemoteExec();
case FlagIdentifier.INCOMPATIBLE_APPLICABLE_LICENSES:
Expand All @@ -138,6 +137,8 @@ boolean flagValue(String flag) {
return experimentalAllowTagsPropagation();
case FlagIdentifier.INCOMPATIBLE_REMOVE_ENABLE_TOOLCHAIN_TYPES:
return incompatibleRemoveEnabledToolchainTypes();
case FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API:
return incompatibleRequireLinkerInputCcApi();
default:
throw new IllegalArgumentException(flag);
}
Expand Down Expand Up @@ -259,6 +260,8 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli

public abstract boolean incompatibleDepsetForLibrariesToLinkGetter();

public abstract boolean incompatibleRequireLinkerInputCcApi();

public abstract boolean incompatibleRestrictStringEscapes();

public abstract boolean experimentalAllowTagsPropagation();
Expand Down Expand Up @@ -339,6 +342,7 @@ public static Builder builderWithDefaults() {
.internalSkylarkFlagTestCanary(false)
.incompatibleDoNotSplitLinkingCmdline(true)
.incompatibleDepsetForLibrariesToLinkGetter(true)
.incompatibleRequireLinkerInputCcApi(false)
.incompatibleRestrictStringEscapes(false)
.incompatibleUseCcConfigureFromRulesCc(false)
.build();
Expand Down Expand Up @@ -430,6 +434,8 @@ public abstract static class Builder {

public abstract Builder incompatibleDepsetForLibrariesToLinkGetter(boolean value);

public abstract Builder incompatibleRequireLinkerInputCcApi(boolean value);

public abstract Builder incompatibleRestrictStringEscapes(boolean value);

public abstract Builder incompatibleUseCcConfigureFromRulesCc(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ public LinkerInputApi<LibraryToLinkApi<FileApi>, FileApi> createLinkerInput(
return null;
}

@Override
public void checkExperimentalCcSharedLibrary(StarlarkThread thread) {}

@Override
public CcLinkingContextApi<FileApi> createCcLinkingInfo(
Object linkerInputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
"--incompatible_restrict_named_params=" + rand.nextBoolean(),
"--incompatible_run_shell_command_string=" + rand.nextBoolean(),
"--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(),
"--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(),
"--incompatible_restrict_string_escapes=" + rand.nextBoolean(),
"--incompatible_use_cc_configure_from_rules_cc=" + rand.nextBoolean(),
"--internal_skylark_flag_test_canary=" + rand.nextBoolean());
Expand Down Expand Up @@ -215,6 +216,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.incompatibleRestrictNamedParams(rand.nextBoolean())
.incompatibleRunShellCommandString(rand.nextBoolean())
.incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean())
.incompatibleRequireLinkerInputCcApi(rand.nextBoolean())
.incompatibleRestrictStringEscapes(rand.nextBoolean())
.incompatibleUseCcConfigureFromRulesCc(rand.nextBoolean())
.internalSkylarkFlagTestCanary(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5827,6 +5827,13 @@ public void testObjectsApiNeverReturningNones() throws Exception {
assertThat((StarlarkList) objects).isEmpty();
}

@Test
public void testIncompatibleRequireLinkerInputCcApi() throws Exception {
setSkylarkSemanticsOptions("--incompatible_require_linker_input_cc_api");
setUpCcLinkingContextTest();
checkError("//a:a", "It may be temporarily re-enabled by setting");
}

private void scratchObjectsProvidingRule() throws IOException {
scratch.file(
"foo/BUILD",
Expand Down

0 comments on commit 8efc50e

Please sign in to comment.