From 06e427215ddb445ddc37b45053e394dc3c050431 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Fri, 31 Jul 2026 14:06:17 -0400 Subject: [PATCH] Starlarkify Objective-C option expansion and tokenization --- cc/common/cc_helper.bzl | 167 +-------------- cc/common/cc_helper_internal.bzl | 191 ++++++++++++++++++ cc/private/cc_common.bzl | 18 +- .../objc/objc_library_local_defines_tests.bzl | 67 ++++++ 4 files changed, 277 insertions(+), 166 deletions(-) diff --git a/cc/common/cc_helper.bzl b/cc/common/cc_helper.bzl index d3ceedbd4..593c720d0 100644 --- a/cc/common/cc_helper.bzl +++ b/cc/common/cc_helper.bzl @@ -25,6 +25,8 @@ load( "path_contains_up_level_references", "should_create_per_object_debug_info", _artifact_category = "artifact_category_names", + _expand = "expand", + _expand_nested_variable = "expand_nested_variable", _extensions = "extensions", _get_cc_runtimes = "get_cc_runtimes", _get_cc_runtimes_copts = "get_cc_runtimes_copts", @@ -32,6 +34,7 @@ load( _package_source_root = "package_source_root", _repository_exec_path = "repository_exec_path", _should_stamp = "should_stamp", + _tokenize = "tokenize", ) load(":cc_info.bzl", "CcInfo") load(":visibility.bzl", "INTERNAL_VISIBILITY") @@ -620,110 +623,6 @@ def _create_strip_action(ctx, cc_toolchain, cpp_config, input, output, feature_c arguments = command_line, ) -def _lookup_var(ctx, additional_vars, var): - expanded_make_var_ctx = ctx.var.get(var) - expanded_make_var_additional = additional_vars.get(var) - if expanded_make_var_additional != None: - return expanded_make_var_additional - if expanded_make_var_ctx != None: - return expanded_make_var_ctx - fail("{}: {} not defined".format(ctx.label, "$(" + var + ")")) - -def _expand_nested_variable(ctx, additional_vars, exp, execpath = True, targets = []): - # If make variable is predefined path variable(like $(location ...)) - # we will expand it first. - if exp.find(" ") != -1: - if not execpath: - if exp.startswith("location"): - exp = exp.replace("location", "rootpath", 1) - data_targets = [] - if ctx.attr.data != None: - data_targets = ctx.attr.data - - # Make sure we do not duplicate targets. - unified_targets_set = {} - for data_target in data_targets: - unified_targets_set[data_target] = True - for target in targets: - unified_targets_set[target] = True - return ctx.expand_location("$({})".format(exp), targets = unified_targets_set.keys()) - - # Recursively expand nested make variables, but since there is no recursion - # in Starlark we will do it via for loop. - unbounded_recursion = True - - # The only way to check if the unbounded recursion is happening or not - # is to have a look at the depth of the recursion. - # 10 seems to be a reasonable number, since it is highly unexpected - # to have nested make variables which are expanding more than 10 times. - for _ in range(10): - exp = _lookup_var(ctx, additional_vars, exp) - if len(exp) >= 3 and exp[0] == "$" and exp[1] == "(" and exp[len(exp) - 1] == ")": - # Try to expand once more. - exp = exp[2:len(exp) - 1] - continue - unbounded_recursion = False - break - - if unbounded_recursion: - fail("potentially unbounded recursion during expansion of {}".format(exp)) - return exp - -def _expand(ctx, expression, additional_make_variable_substitutions, execpath = True, targets = []): - idx = 0 - last_make_var_end = 0 - result = [] - n = len(expression) - for _ in range(n): - if idx >= n: - break - if expression[idx] != "$": - idx += 1 - continue - - idx += 1 - - # We've met $$ pattern, so $ is escaped. - if idx < n and expression[idx] == "$": - idx += 1 - result.append(expression[last_make_var_end:idx - 1]) - last_make_var_end = idx - # We might have found a potential start for Make Variable. - - elif idx < n and expression[idx] == "(": - # Try to find the closing parentheses. - make_var_start = idx - make_var_end = make_var_start - for j in range(idx + 1, n): - if expression[j] == ")": - make_var_end = j - break - - # Note we cannot go out of string's bounds here, - # because of this check. - # If start of the variable is different from the end, - # we found a make variable. - if make_var_start != make_var_end: - # Some clarifications: - # *****$(MAKE_VAR_1)*******$(MAKE_VAR_2)***** - # ^ ^ ^ - # | | | - # last_make_var_end make_var_start make_var_end - result.append(expression[last_make_var_end:make_var_start - 1]) - make_var = expression[make_var_start + 1:make_var_end] - exp = _expand_nested_variable(ctx, additional_make_variable_substitutions, make_var, execpath, targets) - result.append(exp) - - # Update indexes. - idx = make_var_end + 1 - last_make_var_end = idx - - # Add the last substring which would be skipped by for loop. - if last_make_var_end < n: - result.append(expression[last_make_var_end:n]) - - return "".join(result) - def _get_expanded_env(ctx, additional_make_variable_substitutions): if not hasattr(ctx.attr, "env"): fail("could not find rule attribute named: 'env'") @@ -739,66 +638,6 @@ def _get_expanded_env(ctx, additional_make_variable_substitutions): ) return expanded_env -# Implementation of Bourne shell tokenization. -# Tokenizes str and appends result to the options list. -def _tokenize(options, options_string): - token = [] - force_token = False - quotation = "\0" - length = len(options_string) - - # Since it is impossible to modify loop variable inside loop - # in Starlark, and also there is no while loop, I have to - # use this ugly hack. - i = -1 - for _ in range(length): - i += 1 - if i >= length: - break - c = options_string[i] - if quotation != "\0": - # In quotation. - if c == quotation: - # End quotation. - quotation = "\0" - elif c == "\\" and quotation == "\"": - i += 1 - if i == length: - fail("backslash at the end of the string: {}".format(options_string)) - c = options_string[i] - if c != "\\" and c != "\"": - token.append("\\") - token.append(c) - else: - # Regular char, in quotation. - token.append(c) - else: - # Not in quotation. - if c == "'" or c == "\"": - # Begin single double quotation. - quotation = c - force_token = True - elif c == " " or c == "\t": - # Space not quoted. - if force_token or len(token) > 0: - options.append("".join(token)) - token = [] - force_token = False - elif c == "\\": - # Backslash not quoted. - i += 1 - if i == length: - fail("backslash at the end of the string: {}".format(options_string)) - token.append(options_string[i]) - else: - # Regular char, not quoted. - token.append(c) - if quotation != "\0": - fail("unterminated quotation at the end of the string: {}".format(options_string)) - - if force_token or len(token) > 0: - options.append("".join(token)) - def _should_use_pic(ctx, cc_toolchain, feature_configuration): """Whether to use pic files diff --git a/cc/common/cc_helper_internal.bzl b/cc/common/cc_helper_internal.bzl index 6d0cb95f0..4fe49285e 100644 --- a/cc/common/cc_helper_internal.bzl +++ b/cc/common/cc_helper_internal.bzl @@ -40,6 +40,197 @@ def wrap_with_check_private_api(symbol): return callback +def _lookup_var(ctx, additional_vars, var): + expanded_make_var_ctx = ctx.var.get(var) + expanded_make_var_additional = additional_vars.get(var) + if expanded_make_var_additional != None: + return expanded_make_var_additional + if expanded_make_var_ctx != None: + return expanded_make_var_ctx + fail("{}: {} not defined".format(ctx.label, "$(" + var + ")")) + +def expand_nested_variable(ctx, additional_vars, exp, execpath = True, targets = []): + """Expands one Make variable or location expression. + + Args: + ctx: The rule context containing Make variables and prerequisites. + additional_vars: Additional Make-variable substitutions. + exp: The Make-variable name or location expression without surrounding "$()". + execpath: Whether location expressions use execution paths. + targets: Additional targets available to location expressions. + + Returns: + The expanded Make-variable value or location path. + """ + + # If make variable is predefined path variable(like $(location ...)) + # we will expand it first. + if exp.find(" ") != -1: + if not execpath: + if exp.startswith("location"): + exp = exp.replace("location", "rootpath", 1) + data_targets = getattr(ctx.attr, "data", []) or [] + + # Make sure we do not duplicate targets. + unified_targets_set = {} + for data_target in data_targets: + unified_targets_set[data_target] = True + for target in targets: + unified_targets_set[target] = True + return ctx.expand_location("$({})".format(exp), targets = unified_targets_set.keys()) + + # Recursively expand nested make variables, but since there is no recursion + # in Starlark we will do it via for loop. + unbounded_recursion = True + + # The only way to check if the unbounded recursion is happening or not + # is to have a look at the depth of the recursion. + # 10 seems to be a reasonable number, since it is highly unexpected + # to have nested make variables which are expanding more than 10 times. + for _ in range(10): + exp = _lookup_var(ctx, additional_vars, exp) + if len(exp) >= 3 and exp[0] == "$" and exp[1] == "(" and exp[len(exp) - 1] == ")": + # Try to expand once more. + exp = exp[2:len(exp) - 1] + continue + unbounded_recursion = False + break + + if unbounded_recursion: + fail("potentially unbounded recursion during expansion of {}".format(exp)) + return exp + +def expand(ctx, expression, additional_make_variable_substitutions, execpath = True, targets = []): + """Expands Make variables and prerequisite locations in an expression. + + Args: + ctx: The rule context containing Make variables and prerequisites. + expression: The expression containing Make variables or location expressions. + additional_make_variable_substitutions: Additional Make-variable substitutions. + execpath: Whether location expressions use execution paths. + targets: Additional targets available to location expressions. + + Returns: + The expression with Make variables and location expressions expanded. + """ + idx = 0 + last_make_var_end = 0 + result = [] + n = len(expression) + for _ in range(n): + if idx >= n: + break + if expression[idx] != "$": + idx += 1 + continue + + idx += 1 + + # We've met $$ pattern, so $ is escaped. + if idx < n and expression[idx] == "$": + idx += 1 + result.append(expression[last_make_var_end:idx - 1]) + last_make_var_end = idx + # We might have found a potential start for Make Variable. + + elif idx < n and expression[idx] == "(": + # Try to find the closing parentheses. + make_var_start = idx + make_var_end = make_var_start + for j in range(idx + 1, n): + if expression[j] == ")": + make_var_end = j + break + + # Note we cannot go out of string's bounds here, + # because of this check. + # If start of the variable is different from the end, + # we found a make variable. + if make_var_start != make_var_end: + # Some clarifications: + # *****$(MAKE_VAR_1)*******$(MAKE_VAR_2)***** + # ^ ^ ^ + # | | | + # last_make_var_end make_var_start make_var_end + result.append(expression[last_make_var_end:make_var_start - 1]) + make_var = expression[make_var_start + 1:make_var_end] + exp = expand_nested_variable(ctx, additional_make_variable_substitutions, make_var, execpath, targets) + result.append(exp) + + # Update indexes. + idx = make_var_end + 1 + last_make_var_end = idx + + # Add the last substring which would be skipped by for loop. + if last_make_var_end < n: + result.append(expression[last_make_var_end:n]) + + return "".join(result) + +def tokenize(options, options_string): + """Appends Bourne-shell tokens from options_string to options. + + Args: + options: The list receiving the parsed shell tokens. + options_string: The shell-tokenized string. + """ + token = [] + force_token = False + quotation = "\0" + length = len(options_string) + + # Since it is impossible to modify loop variable inside loop + # in Starlark, and also there is no while loop, I have to + # use this ugly hack. + i = -1 + for _ in range(length): + i += 1 + if i >= length: + break + c = options_string[i] + if quotation != "\0": + # In quotation. + if c == quotation: + # End quotation. + quotation = "\0" + elif c == "\\" and quotation == "\"": + i += 1 + if i == length: + fail("backslash at the end of the string: {}".format(options_string)) + c = options_string[i] + if c != "\\" and c != "\"": + token.append("\\") + token.append(c) + else: + # Regular char, in quotation. + token.append(c) + else: + # Not in quotation. + if c == "'" or c == "\"": + # Begin single double quotation. + quotation = c + force_token = True + elif c == " " or c == "\t": + # Space not quoted. + if force_token or len(token) > 0: + options.append("".join(token)) + token = [] + force_token = False + elif c == "\\": + # Backslash not quoted. + i += 1 + if i == length: + fail("backslash at the end of the string: {}".format(options_string)) + token.append(options_string[i]) + else: + # Regular char, not quoted. + token.append(c) + if quotation != "\0": + fail("unterminated quotation at the end of the string: {}".format(options_string)) + + if force_token or len(token) > 0: + options.append("".join(token)) + CPP_SOURCE_TYPE_HEADER = "HEADER" CPP_SOURCE_TYPE_SOURCE = "SOURCE" CPP_SOURCE_TYPE_CLIF_INPUT_PROTO = "CLIF_INPUT_PROTO" diff --git a/cc/private/cc_common.bzl b/cc/private/cc_common.bzl index d78312c6a..2795fd4e8 100644 --- a/cc/private/cc_common.bzl +++ b/cc/private/cc_common.bzl @@ -17,6 +17,8 @@ load( "//cc/common:cc_helper_internal.bzl", _CREATE_COMPILE_ACTION_API_ALLOWLISTED_PACKAGES = "CREATE_COMPILE_ACTION_API_ALLOWLISTED_PACKAGES", _PRIVATE_STARLARKIFICATION_ALLOWLIST = "PRIVATE_STARLARKIFICATION_ALLOWLIST", + _expand = "expand", + _tokenize = "tokenize", ) load("//cc/private:cc_info.bzl", "CcNativeLibraryInfo", "create_compilation_context", "create_debug_context", "create_linking_context", "create_module_map", "merge_cc_infos", "merge_compilation_contexts", "merge_debug_context", "merge_linking_contexts") load("//cc/private:cc_internal.bzl", _cc_internal = "cc_internal") @@ -679,9 +681,21 @@ def _absolute_symlink(*, ctx, output, target_path, progress_message): progress_message = progress_message, ) -def _objc_expand_and_tokenize(**kwargs): +# buildifier: disable=unused-variable +def _objc_expand_and_tokenize(*, ctx, attr, flags = []): _cc_internal.check_private_api(allowlist = _PRIVATE_STARLARKIFICATION_ALLOWLIST) - return _cc_internal.expand_and_tokenize(**kwargs) + if not flags: + return flags + + targets = [] + for attribute in ["srcs", "non_arc_srcs", "hdrs", "data", "additional_linker_inputs"]: + targets.extend(getattr(ctx.attr, attribute, [])) + + expanded_flags = [] + for flag in flags: + expanded_flag = _expand(ctx, flag, {}, targets = targets) + _tokenize(expanded_flags, expanded_flag) + return expanded_flags def _create_linkstamp(linkstamp, headers): _cc_internal.check_private_api(allowlist = _PRIVATE_STARLARKIFICATION_ALLOWLIST) diff --git a/tests/cc/objc/objc_library_local_defines_tests.bzl b/tests/cc/objc/objc_library_local_defines_tests.bzl index 8285107fd..65726f96d 100644 --- a/tests/cc/objc/objc_library_local_defines_tests.bzl +++ b/tests/cc/objc/objc_library_local_defines_tests.bzl @@ -35,6 +35,71 @@ def _test_local_defines_in_compile_action_impl(env, target): env.expect.that_collection(compile_actions).has_size(1) env.expect.that_collection(compile_actions[0].argv).contains("-DLOCAL_DEF=1") +def _test_local_defines_expand_and_tokenize(name, **kwargs): + util.helper_target( + objc_library, + name = name + "_lib", + srcs = ["foo.m"], + local_defines = [ + "QUOTED='two words' SECOND=1", + "ESCAPED=one\\ two", + "MODE=$(COMPILATION_MODE)", + "DOLLAR=$$PWD", + ], + ) + cc_analysis_test( + name = name, + impl = _test_local_defines_expand_and_tokenize_impl, + target = name + "_lib", + with_action_configs = _OBJC_ACTION_CONFIGS, + test_features = [FEATURE_NAMES.preprocessor_defines], + **kwargs + ) + +def _test_local_defines_expand_and_tokenize_impl(env, target): + compile_actions = [a for a in target.actions if a.mnemonic == "ObjcCompile"] + env.expect.that_collection(compile_actions).has_size(1) + argv = env.expect.that_collection(compile_actions[0].argv) + argv.contains("-DQUOTED=two words") + argv.contains("-DSECOND=1") + argv.contains("-DESCAPED=one two") + argv.contains("-DMODE=fastbuild") + argv.contains("-DDOLLAR=$PWD") + +def _test_local_defines_expand_prerequisite_locations(name, **kwargs): + util.helper_target( + objc_library, + name = name + "_lib", + srcs = ["foo.m"], + non_arc_srcs = ["non_arc.m"], + hdrs = ["objc_header.h"], + data = ["objc_data.txt"], + local_defines = [ + "SOURCE=$(location foo.m)", + "NON_ARC=$(location non_arc.m)", + "HEADER=$(location objc_header.h)", + "DATA=$(location objc_data.txt)", + ], + ) + cc_analysis_test( + name = name, + impl = _test_local_defines_expand_prerequisite_locations_impl, + target = name + "_lib", + with_action_configs = _OBJC_ACTION_CONFIGS, + test_features = [FEATURE_NAMES.preprocessor_defines], + **kwargs + ) + +def _test_local_defines_expand_prerequisite_locations_impl(env, target): + compile_actions = [a for a in target.actions if a.mnemonic == "ObjcCompile"] + env.expect.that_collection(compile_actions).has_size(2) + for action in compile_actions: + argv = env.expect.that_collection(action.argv) + argv.contains("-DSOURCE={}/foo.m".format(target.label.package)) + argv.contains("-DNON_ARC={}/non_arc.m".format(target.label.package)) + argv.contains("-DHEADER={}/objc_header.h".format(target.label.package)) + argv.contains("-DDATA={}/objc_data.txt".format(target.label.package)) + def _test_local_defines_not_in_cc_info(name, **kwargs): """local_defines should not be in CcInfo.compilation_context.defines.""" util.helper_target( @@ -91,6 +156,8 @@ def objc_library_local_defines_tests(name): if bazel_features.cc.cc_common_is_in_rules_cc: tests.extend([ + _test_local_defines_expand_and_tokenize, + _test_local_defines_expand_prerequisite_locations, _test_local_defines_in_compile_action, _test_local_defines_not_in_cc_info, _test_local_defines_not_propagated_to_dependent,