Skip to content

Commit

Permalink
Add conlyopts and cxxopts attributes to cc rules
Browse files Browse the repository at this point in the history
The inability to pass C or C++ specific compiler flags to targets that
contain a mix of those sources is a common sticking point for new users.
These mirror the global `--conlyopt` and `--cxxopt` flags but at the
target level.

Fixes #22041

RELNOTES: Add conlyopts and cxxopts attributes to cc rules

Closes #23792.

PiperOrigin-RevId: 682144094
Change-Id: I0fe8b728c493652d875ce6a6dd2a9989c36b1f24
  • Loading branch information
keith authored and copybara-github committed Oct 4, 2024
1 parent 5c82594 commit 5854788
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ public CcCompilationContext getCcCompilationContext() {
private final LinkedHashMap<Artifact, CppSource> compilationUnitSources = new LinkedHashMap<>();
private final LinkedHashMap<Artifact, CppSource> moduleInterfaceSources = new LinkedHashMap<>();
private ImmutableList<String> copts = ImmutableList.of();
private ImmutableList<String> conlyopts = ImmutableList.of();
private ImmutableList<String> cxxopts = ImmutableList.of();
private CoptsFilter coptsFilter = CoptsFilter.alwaysPasses();
private final Set<String> defines = new LinkedHashSet<>();
private final Set<String> localDefines = new LinkedHashSet<>();
Expand Down Expand Up @@ -615,6 +617,18 @@ public void setCoptsFilter(CoptsFilter coptsFilter) {
this.coptsFilter = Preconditions.checkNotNull(coptsFilter);
}

@CanIgnoreReturnValue
public CcCompilationHelper setConlyopts(ImmutableList<String> copts) {
this.conlyopts = Preconditions.checkNotNull(copts);
return this;
}

@CanIgnoreReturnValue
public CcCompilationHelper setCxxopts(ImmutableList<String> copts) {
this.cxxopts = Preconditions.checkNotNull(copts);
return this;
}

/**
* Adds the given defines to the compiler command line of this target as well as its dependent
* targets.
Expand Down Expand Up @@ -1270,9 +1284,21 @@ public static ImmutableList<String> getCoptsFromOptions(

private ImmutableList<String> getCopts(Artifact sourceFile, Label sourceLabel) {
ImmutableList.Builder<String> coptsList = ImmutableList.builder();
coptsList.addAll(
getCoptsFromOptions(cppConfiguration, semantics, sourceFile.getExecPathString()));
String sourceFilename = sourceFile.getExecPathString();
coptsList.addAll(getCoptsFromOptions(cppConfiguration, semantics, sourceFilename));
coptsList.addAll(copts);

if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
coptsList.addAll(conlyopts);
}

if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
|| CppFileTypes.CPP_HEADER.matches(sourceFilename)
|| CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
|| CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
coptsList.addAll(cxxopts);
}

if (sourceFile != null && sourceLabel != null) {
coptsList.addAll(collectPerFileCopts(sourceFile, sourceLabel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,8 @@ public Tuple compile(
String includePrefix,
String stripIncludePrefix,
Sequence<?> userCompileFlags, // <String> expected
Sequence<?> conlyFlags, // <String> expected
Sequence<?> cxxFlags, // <String> expected
Sequence<?> ccCompilationContexts, // <CcCompilationContext> expected
Object implementationCcCompilationContextsObject,
String name,
Expand Down Expand Up @@ -2216,6 +2218,8 @@ public Tuple compile(
.setCopts(
ImmutableList.copyOf(
Sequence.cast(userCompileFlags, String.class, "user_compile_flags")))
.setConlyopts(ImmutableList.copyOf(Sequence.cast(conlyFlags, String.class, "conly_flags")))
.setCxxopts(ImmutableList.copyOf(Sequence.cast(cxxFlags, String.class, "cxx_flags")))
.addAdditionalCompilationInputs(
Sequence.cast(additionalInputs, Artifact.class, "additional_inputs"))
.addAdditionalInputs(nonCompilationAdditionalInputs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,18 @@ default void compilerFlagExists() {}
positional = false,
named = true,
defaultValue = "[]"),
@Param(
name = "conly_flags",
doc = "Additional list of compilation options for C compiles.",
positional = false,
named = true,
defaultValue = "[]"),
@Param(
name = "cxx_flags",
doc = "Additional list of compilation options for C++ compiles.",
positional = false,
named = true,
defaultValue = "[]"),
@Param(
name = "compilation_contexts",
doc = "Headers from dependencies used for compilation.",
Expand Down Expand Up @@ -395,6 +407,8 @@ Tuple compile(
String includePrefix,
String stripIncludePrefix,
Sequence<?> userCompileFlags, // <String> expected
Sequence<?> conlyFlags, // <String> expected
Sequence<?> cxxFlags, // <String> expected
Sequence<?> ccCompilationContexts, // <CcCompilationContext> expected
Object implementationCcCompilationContextsObject,
String name,
Expand Down
12 changes: 11 additions & 1 deletion src/main/starlark/builtins_bzl/common/cc/attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ is prepended with <code>-D</code> and added to the compile command line for this
but not to its dependents.
"""),
"copts": attr.string_list(doc = """
Add these options to the C++ compilation command.
Add these options to the C/C++ compilation command.
Subject to <a href="${link make-variables}">"Make variable"</a> substitution and
<a href="${link common-definitions#sh-tokenization}">Bourne shell tokenization</a>.
<p>
Expand All @@ -178,6 +178,16 @@ Subject to <a href="${link make-variables}">"Make variable"</a> substitution and
<code>no_copts_tokenization</code>, Bourne shell tokenization applies only to strings
that consist of a single "Make" variable.
</p>
"""),
"conlyopts": attr.string_list(doc = """
Add these options to the C compilation command.
Subject to <a href="${link make-variables}">"Make variable"</a> substitution and
<a href="${link common-definitions#sh-tokenization}">Bourne shell tokenization</a>.
"""),
"cxxopts": attr.string_list(doc = """
Add these options to the C++ compilation command.
Subject to <a href="${link make-variables}">"Make variable"</a> substitution and
<a href="${link common-definitions#sh-tokenization}">Bourne shell tokenization</a>.
"""),
"hdrs_check": attr.string(
doc = "Deprecated, no-op.",
Expand Down
4 changes: 3 additions & 1 deletion src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,9 @@ def cc_binary_impl(ctx, additional_linkopts, force_linkstatic = False):
actions = ctx.actions,
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
user_compile_flags = runtimes_copts + cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = runtimes_copts + cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "copts"),
conly_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "conlyopts"),
cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"),
defines = cc_helper.defines(ctx, additional_make_variable_substitutions),
local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps),
system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions),
Expand Down
4 changes: 4 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,8 @@ def _compile(
include_prefix = "",
strip_include_prefix = "",
user_compile_flags = [],
conly_flags = [],
cxx_flags = [],
compilation_contexts = [],
implementation_compilation_contexts = _UNBOUND,
disallow_pic_outputs = False,
Expand Down Expand Up @@ -760,6 +762,8 @@ def _compile(
include_prefix = include_prefix,
strip_include_prefix = strip_include_prefix,
user_compile_flags = user_compile_flags,
conly_flags = conly_flags,
cxx_flags = cxx_flags,
compilation_contexts = compilation_contexts,
implementation_compilation_contexts = implementation_compilation_contexts,
disallow_pic_outputs = disallow_pic_outputs,
Expand Down
8 changes: 4 additions & 4 deletions src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -924,10 +924,10 @@ def _expand_make_variables_for_copts(ctx, tokenization, unexpanded_tokens, addit
tokens.append(_expand(ctx, token, additional_make_variable_substitutions, targets = targets))
return tokens

def _get_copts(ctx, feature_configuration, additional_make_variable_substitutions):
if not hasattr(ctx.attr, "copts"):
fail("could not find rule attribute named: 'copts'")
attribute_copts = ctx.attr.copts
def _get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "copts"):
if not hasattr(ctx.attr, attr):
fail("could not find rule attribute named: '{}'".format(attr))
attribute_copts = getattr(ctx.attr, attr)
tokenization = not (cc_common.is_enabled(feature_configuration = feature_configuration, feature_name = "no_copts_tokenization") or "no_copts_tokenization" in ctx.features)
expanded_attribute_copts = _expand_make_variables_for_copts(ctx, tokenization, attribute_copts, additional_make_variable_substitutions)
return expanded_attribute_copts
Expand Down
4 changes: 3 additions & 1 deletion src/main/starlark/builtins_bzl/common/cc/cc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ def _cc_library_impl(ctx):
name = ctx.label.name,
cc_toolchain = cc_toolchain,
feature_configuration = feature_configuration,
user_compile_flags = runtimes_copts + cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = runtimes_copts + cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "copts"),
conly_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "conlyopts"),
cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"),
defines = cc_helper.defines(ctx, additional_make_variable_substitutions),
local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps + ctx.attr.implementation_deps),
system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,48 @@ public void testPresenceOfUserCompileFlags() throws Exception {
assertThat(copts).contains("-foo");
}

@Test
public void testPresenceOfConlyFlags() throws Exception {
useConfiguration(
"--conlyopt=-foo", "--cxxopt=-not-passed", "--per_file_copt=//x:bin@-per-file");

scratch.file(
"x/BUILD",
"cc_binary(name = 'bin', srcs = ['bin.c'], copts = ['-bar'], conlyopts = ['-baz'], cxxopts"
+ " = ['-not-passed'])");
scratch.file("x/bin.c");

CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin");

ImmutableList<String> copts =
CcToolchainVariables.toStringList(
variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP);
assertThat(copts)
.containsExactlyElementsIn(ImmutableList.<String>of("-foo", "-bar", "-baz", "-per-file"))
.inOrder();
}

@Test
public void testCxxFlagsOrder() throws Exception {
useConfiguration(
"--cxxopt=-foo", "--conlyopt=-not-passed", "--per_file_copt=//x:bin@-per-file");

scratch.file(
"x/BUILD",
"cc_binary(name = 'bin', srcs = ['bin.cc'], copts = ['-bar'], cxxopts = ['-baz'], conlyopts"
+ " = ['-not-passed'])");
scratch.file("x/bin.cc");

CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin");

ImmutableList<String> copts =
CcToolchainVariables.toStringList(
variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP);
assertThat(copts)
.containsExactlyElementsIn(ImmutableList.<String>of("-foo", "-bar", "-baz", "-per-file"))
.inOrder();
}

@Test
public void testPerFileCoptsAreInUserCompileFlags() throws Exception {
scratch.file("x/BUILD", "cc_binary(name = 'bin', srcs = ['bin.cc'])");
Expand Down

0 comments on commit 5854788

Please sign in to comment.