Skip to content

Commit

Permalink
[7.4.0] 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 bazelbuild#22041

RELNOTES: Add conlyopts and cxxopts attributes to cc rules

Closes bazelbuild#23840

PiperOrigin-RevId: 682144094
Change-Id: I0fe8b728c493652d875ce6a6dd2a9989c36b1f24

(cherry picked from commit 5854788)
  • Loading branch information
keith committed Oct 8, 2024
1 parent e2193ee commit 0ca68c4
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ public CcCompilationContext getCcCompilationContext() {
private final List<CppModuleMap> additionalCppModuleMaps = new ArrayList<>();
private final LinkedHashMap<Artifact, CppSource> compilationUnitSources = 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 @@ -594,6 +596,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 @@ -1247,9 +1261,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 @@ -2156,6 +2156,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 @@ -2310,6 +2312,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 @@ -242,6 +242,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 @@ -396,6 +408,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
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 @@ -633,7 +633,9 @@ def cc_binary_impl(ctx, additional_linkopts):
actions = ctx.actions,
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
user_compile_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = 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
2 changes: 2 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary_attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ cc_binary_attrs = {
),
"linkopts": attr.string_list(),
"copts": attr.string_list(),
"conlyopts": attr.string_list(),
"cxxopts": attr.string_list(),
"defines": attr.string_list(),
"local_defines": attr.string_list(),
"includes": attr.string_list(),
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 @@ -705,6 +705,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 @@ -785,6 +787,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
6 changes: 5 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 @@ -55,7 +55,9 @@ def _cc_library_impl(ctx):
name = ctx.label.name,
cc_toolchain = cc_toolchain,
feature_configuration = feature_configuration,
user_compile_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = 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 Expand Up @@ -581,6 +583,8 @@ attrs = {
"includes": attr.string_list(),
"defines": attr.string_list(),
"copts": attr.string_list(),
"conlyopts": attr.string_list(),
"cxxopts": attr.string_list(),
"hdrs_check": attr.string(default = cc_internal.default_hdrs_check_computed_default()),
"local_defines": attr.string_list(),
"deps": attr.label_list(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,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 0ca68c4

Please sign in to comment.