From 1b4c81fa6939262a800e9c7e1a7469e3b66d4e03 Mon Sep 17 00:00:00 2001 From: Jon Meow <46229924+jonmeow@users.noreply.github.com> Date: Fri, 7 Jan 2022 14:44:40 -0800 Subject: [PATCH] Add script for generating missing C++ deps (#1012) Co-authored-by: Geoff Romer --- executable_semantics/ast/BUILD | 13 ++ executable_semantics/interpreter/BUILD | 16 ++ migrate_cpp/cpp_refactoring/BUILD | 20 +- scripts/BUILD | 13 ++ scripts/fix_cc_deps.py | 244 +++++++++++++++++++++++++ toolchain/common/BUILD | 1 + toolchain/parser/BUILD | 2 + toolchain/parser/parse_test_helpers.h | 2 +- 8 files changed, 307 insertions(+), 4 deletions(-) create mode 100755 scripts/fix_cc_deps.py diff --git a/executable_semantics/ast/BUILD b/executable_semantics/ast/BUILD index 21632090e132..05734ff50fd3 100644 --- a/executable_semantics/ast/BUILD +++ b/executable_semantics/ast/BUILD @@ -10,6 +10,7 @@ cc_library( deps = [ ":declaration", ":library_name", + "//executable_semantics/common:nonnull", ], ) @@ -22,6 +23,7 @@ cc_library( ], deps = [ ":source_location", + "@llvm-project//llvm:Support", ], ) @@ -94,6 +96,7 @@ cc_library( deps = [ ":ast_node", ":paren_contents", + ":source_location", ":static_scope", ":value_category", "//common:indirect_value", @@ -110,7 +113,9 @@ cc_test( deps = [ ":expression", ":paren_contents", + "//executable_semantics/common:arena", "@com_google_googletest//:gtest_main", + "@llvm-project//llvm:Support", ], ) @@ -119,9 +124,12 @@ cc_library( srcs = ["member.cpp"], hdrs = ["member.h"], deps = [ + ":expression", ":pattern", ":source_location", "//common:ostream", + "//executable_semantics/common:arena", + "@llvm-project//llvm:Support", ], ) @@ -160,8 +168,10 @@ cc_test( name = "pattern_test", srcs = ["pattern_test.cpp"], deps = [ + ":expression", ":paren_contents", ":pattern", + "//executable_semantics/common:arena", "@com_google_googletest//:gtest_main", "@llvm-project//llvm:Support", ], @@ -175,8 +185,10 @@ cc_library( ":ast_node", ":source_location", ":value_category", + "//common:check", "//executable_semantics/common:arena", "//executable_semantics/common:error", + "//executable_semantics/common:nonnull", ], ) @@ -198,6 +210,7 @@ cc_library( ":expression", ":pattern", ":source_location", + ":static_scope", ":value_category", "//common:check", "//common:ostream", diff --git a/executable_semantics/interpreter/BUILD b/executable_semantics/interpreter/BUILD index 63e98856f0d9..ca7edfdea234 100644 --- a/executable_semantics/interpreter/BUILD +++ b/executable_semantics/interpreter/BUILD @@ -22,12 +22,15 @@ cc_library( ":field_path", ":heap_allocation_interface", ":stack", + "//common:check", "//common:ostream", "//executable_semantics/ast:declaration", "//executable_semantics/ast:expression", + "//executable_semantics/ast:pattern", "//executable_semantics/ast:statement", "//executable_semantics/common:arena", "//executable_semantics/common:error", + "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], ) @@ -70,7 +73,10 @@ cc_library( ":resolve_control_flow", ":resolve_names", ":type_checker", + "//common:check", + "//common:ostream", "//executable_semantics/ast", + "//executable_semantics/common:arena", ], ) @@ -93,6 +99,7 @@ cc_library( ":heap_allocation_interface", "//common:ostream", "//executable_semantics/ast:source_location", + "//executable_semantics/common:error", "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], @@ -120,11 +127,14 @@ cc_library( ":action_stack", ":address", ":heap", + ":stack", "//common:check", "//common:ostream", "//executable_semantics/ast:declaration", "//executable_semantics/ast:expression", + "//executable_semantics/ast:pattern", "//executable_semantics/common:arena", + "//executable_semantics/common:error", "@llvm-project//llvm:Support", ], ) @@ -138,6 +148,8 @@ cc_library( "//executable_semantics/ast", "//executable_semantics/ast:declaration", "//executable_semantics/ast:statement", + "//executable_semantics/common:error", + "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], ) @@ -155,6 +167,7 @@ cc_library( "//executable_semantics/ast:pattern", "//executable_semantics/ast:statement", "//executable_semantics/ast:static_scope", + "//executable_semantics/common:arena", "@llvm-project//llvm:Support", ], ) @@ -170,6 +183,7 @@ cc_library( srcs = ["type_checker.cpp"], hdrs = ["type_checker.h"], deps = [ + ":action_and_value", ":dictionary", ":interpreter", "//common:ostream", @@ -178,6 +192,8 @@ cc_library( "//executable_semantics/ast:expression", "//executable_semantics/ast:statement", "//executable_semantics/common:arena", + "//executable_semantics/common:error", + "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], ) diff --git a/migrate_cpp/cpp_refactoring/BUILD b/migrate_cpp/cpp_refactoring/BUILD index 0deb1406bd6b..6963856b0cfe 100644 --- a/migrate_cpp/cpp_refactoring/BUILD +++ b/migrate_cpp/cpp_refactoring/BUILD @@ -10,6 +10,7 @@ cc_binary( deps = [ ":fn_inserter", ":for_range", + ":matcher", ":var_decl", "@llvm-project//clang:tooling", ], @@ -24,6 +25,8 @@ cc_library( ], deps = [ "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:basic", + "@llvm-project//clang:lex", "@llvm-project//clang:tooling_core", ], ) @@ -37,6 +40,7 @@ cc_library( "@com_google_googletest//:gtest", "@llvm-project//clang:ast_matchers", "@llvm-project//clang:tooling", + "@llvm-project//clang:tooling_core", ], ) @@ -46,7 +50,10 @@ cc_library( name = "fn_inserter", srcs = ["fn_inserter.cpp"], hdrs = ["fn_inserter.h"], - deps = [":matcher"], + deps = [ + ":matcher", + "@llvm-project//clang:ast_matchers", + ], ) cc_test( @@ -64,7 +71,10 @@ cc_library( name = "for_range", srcs = ["for_range.cpp"], hdrs = ["for_range.h"], - deps = [":matcher"], + deps = [ + ":matcher", + "@llvm-project//clang:ast_matchers", + ], ) cc_test( @@ -82,7 +92,11 @@ cc_library( name = "var_decl", srcs = ["var_decl.cpp"], hdrs = ["var_decl.h"], - deps = [":matcher"], + deps = [ + ":matcher", + "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:type_nodes_gen", + ], ) cc_test( diff --git a/scripts/BUILD b/scripts/BUILD index df4e5915c3f3..5f6fbd96f6e7 100644 --- a/scripts/BUILD +++ b/scripts/BUILD @@ -16,3 +16,16 @@ mypy_test( include_imports = True, deps = [":create_compdb"], ) + +# Note that this Python script is intended to be run directly, and not through +# Bazel. We have a rule for it so we can type check it. +py_binary( + name = "fix_cc_deps", + srcs = ["fix_cc_deps.py"], +) + +mypy_test( + name = "fix_cc_deps_mypy_test", + include_imports = True, + deps = [":fix_cc_deps"], +) diff --git a/scripts/fix_cc_deps.py b/scripts/fix_cc_deps.py new file mode 100755 index 000000000000..34f216adf62c --- /dev/null +++ b/scripts/fix_cc_deps.py @@ -0,0 +1,244 @@ +#!/usr/bin/env python3 + +"""Automatically fixes bazel C++ dependencies. + +Bazel has some support for detecting when an include refers to a missing +dependency. However, the ideal state is that a given build target depends +directly on all #include'd headers, and Bazel doesn't enforce that. This +automates the addition for technical correctness. +""" + +__copyright__ = """ +Part of the Carbon Language project, under the Apache License v2.0 with LLVM +Exceptions. See /LICENSE for license information. +SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +""" + +import os +import re +import shutil +import subprocess +from pathlib import Path +from typing import Callable, Dict, List, NamedTuple, Set, Tuple +from xml.etree import ElementTree + + +# Maps external repository names to a method translating bazel labels to file +# paths for that repository. +EXTERNAL_REPOS: Dict[str, Callable[[str], str]] = { + "@llvm-project": lambda x: re.sub("^(.*:(lib|include))/", "", x) +} + + +class Rule(NamedTuple): + # For cc_* rules: + # The hdrs + textual_hdrs attributes, as relative paths to the file. + hdrs: Set[str] + # The srcs attribute, as relative paths to the file. + srcs: Set[str] + # The deps attribute, as full bazel labels. + deps: Set[str] + + # For genrules: + # The outs attribute, as relative paths to the file. + outs: Set[str] + + +def locate_bazel() -> str: + """Returns the bazel command. + + We use the `BAZEL` environment variable if present. If not, then we try to + use `bazelisk` and then `bazel`. + """ + bazel = os.environ.get("BAZEL") + if bazel: + return bazel + + if shutil.which("bazelisk"): + return "bazelisk" + + if shutil.which("bazel"): + return "bazel" + + exit("Unable to run Bazel") + + +def remap_file(label: str) -> str: + """Remaps a bazel label to a file.""" + repo, _, path = label.partition("//") + if not repo: + return path.replace(":", "/") + assert repo in EXTERNAL_REPOS, repo + return EXTERNAL_REPOS[repo](path) + exit(f"Don't know how to remap label '{label}'") + + +def get_bazel_list(list_child: ElementTree.Element, is_file: bool) -> Set[str]: + """Returns the contents of a bazel list. + + The return will normally be the full label, unless `is_file` is set, in + which case the label will be translated to the underlying file. + """ + results: Set[str] = set() + for label in list_child: + assert label.tag in ("label", "output"), label.tag + value = label.attrib["value"] + if is_file: + value = remap_file(value) + results.add(value) + return results + + +def get_rules(targets: str, keep_going: bool) -> Dict[str, Rule]: + """Queries the specified targets, returning the found rules. + + keep_going will be set to true for external repositories, where sometimes we + see query errors. + + The return maps rule names to rule data. + """ + args = [ + "bazel", + "query", + "--output=xml", + f"kind('(cc_binary|cc_library|cc_test|genrule)', set({targets}))", + ] + if keep_going: + args.append("--keep_going") + p = subprocess.run( + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8" + ) + # 3 indicates incomplete results from --keep_going, which is fine here. + if p.returncode not in {0, 3}: + print(p.stderr) + exit(f"bazel query returned {p.returncode}") + rules: Dict[str, Rule] = {} + for rule_xml in ElementTree.fromstring(p.stdout): + assert rule_xml.tag == "rule", rule_xml.tag + rule_name = rule_xml.attrib["name"] + hdrs: Set[str] = set() + srcs: Set[str] = set() + deps: Set[str] = set() + outs: Set[str] = set() + rule_class = rule_xml.attrib["class"] + for list_child in rule_xml.findall("list"): + list_name = list_child.attrib["name"] + if rule_class in ("cc_library", "cc_binary", "cc_test"): + if list_name in ("hdrs", "textual_hdrs"): + hdrs = hdrs.union(get_bazel_list(list_child, True)) + elif list_name == "srcs": + srcs = get_bazel_list(list_child, True) + elif list_name == "deps": + deps = get_bazel_list(list_child, False) + elif rule_class == "genrule": + if list_name == "outs": + outs = get_bazel_list(list_child, True) + else: + exit(f"unexpected rule type: {rule_class}") + rules[rule_name] = Rule(hdrs, srcs, deps, outs) + return rules + + +def map_headers( + header_to_rule_map: Dict[str, Set[str]], rules: Dict[str, Rule] +) -> None: + """Accumulates headers provided by rules into the map. + + The map maps header paths to rule names. + """ + for rule_name, rule in rules.items(): + for header in rule.hdrs: + if header in header_to_rule_map: + header_to_rule_map[header].add(rule_name) + else: + header_to_rule_map[header] = {rule_name} + + +def get_missing_deps( + header_to_rule_map: Dict[str, Set[str]], + generated_files: Set[str], + rule: Rule, +) -> Tuple[Set[str], bool]: + """Returns missing dependencies for the rule. + + On return, the set is dependency labels that should be added; the bool + indicates whether some where omitted due to ambiguity. + """ + missing_deps: Set[str] = set() + ambiguous = False + rule_files = rule.hdrs.union(rule.srcs) + for source_file in rule_files: + if source_file in generated_files: + continue + with open(source_file, "r") as f: + for header in re.findall( + r'^#include "([^"]+)"', f.read(), re.MULTILINE + ): + if header in rule_files: + continue + if header not in header_to_rule_map: + exit( + f"Missing rule for #include '{header}' in " + f"'{source_file}'" + ) + dep_choices = header_to_rule_map[header] + if not dep_choices.intersection(rule.deps): + if len(dep_choices) > 1: + print( + f"Ambiguous dependency choice for #include " + f"'{header}' in '{source_file}': " + f"{', '.join(dep_choices)}" + ) + ambiguous = True + missing_deps.add(dep_choices.pop()) + return missing_deps, ambiguous + + +def main() -> None: + # Change the working directory to the repository root so that the remaining + # operations reliably operate relative to that root. + os.chdir(Path(__file__).parent.parent) + + print("Querying bazel for Carbon targets...") + carbon_rules = get_rules("//...", False) + print("Querying bazel for external targets...") + external_repo_query = " ".join([f"{repo}//..." for repo in EXTERNAL_REPOS]) + external_rules = get_rules(external_repo_query, True) + + print("Building header map...") + header_to_rule_map: Dict[str, Set[str]] = {} + map_headers(header_to_rule_map, carbon_rules) + map_headers(header_to_rule_map, external_rules) + + print("Building generated file list...") + generated_files: Set[str] = set() + for rule in carbon_rules.values(): + generated_files = generated_files.union(rule.outs) + + print("Parsing headers from source files...") + all_missing_deps: List[Tuple[str, Set[str]]] = [] + any_ambiguous = False + for rule_name, rule in carbon_rules.items(): + missing_deps, ambiguous = get_missing_deps( + header_to_rule_map, generated_files, rule + ) + if missing_deps: + all_missing_deps.append((rule_name, missing_deps)) + if ambiguous: + any_ambiguous = True + if any_ambiguous: + exit("Stopping due to ambiguous dependency choices.") + + print("Fixing dependencies...") + SEPARATOR = "\n- " + for rule_name, missing_deps in sorted(all_missing_deps): + friendly_missing_deps = SEPARATOR.join(missing_deps) + print(f"Adding deps to {rule_name}:{SEPARATOR}{friendly_missing_deps}") + args = ["buildozer", f"add deps {' '.join(missing_deps)}", rule_name] + subprocess.check_call(args) + + print("Done!") + + +if __name__ == "__main__": + main() diff --git a/toolchain/common/BUILD b/toolchain/common/BUILD index 0bbde85550a9..c1fc155b6da0 100644 --- a/toolchain/common/BUILD +++ b/toolchain/common/BUILD @@ -12,5 +12,6 @@ cc_library( deps = [ "//common:ostream", "@com_google_googletest//:gtest", + "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/parser/BUILD b/toolchain/parser/BUILD index ab2ef9e222fe..5a5191f74f2d 100644 --- a/toolchain/parser/BUILD +++ b/toolchain/parser/BUILD @@ -37,6 +37,7 @@ cc_library( ":parse_node_kind", ":precedence", "//common:check", + "//common:ostream", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/lexer:token_kind", "//toolchain/lexer:tokenized_buffer", @@ -51,6 +52,7 @@ cc_library( deps = [ ":parse_node_kind", ":parse_tree", + "//common:check", "//toolchain/lexer:tokenized_buffer", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", diff --git a/toolchain/parser/parse_test_helpers.h b/toolchain/parser/parse_test_helpers.h index bccfee8b96c8..1141c42df5be 100644 --- a/toolchain/parser/parse_test_helpers.h +++ b/toolchain/parser/parse_test_helpers.h @@ -315,7 +315,7 @@ auto MatchNode(Args... args) -> ExpectedNode { auto Match##kind(Args... args)->ExpectedNode { \ return MatchNode(ParseNodeKind::kind(), std::move(args)...); \ } -#include "parse_node_kind.def" +#include "toolchain/parser/parse_node_kind.def" // Helper for matching a designator `lhs.rhs`. inline auto MatchDesignator(ExpectedNode lhs, std::string rhs) -> ExpectedNode {