From be4ff14bca906d104f76777cae65c78ff4450d56 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 3 Jul 2025 19:19:08 +0100 Subject: [PATCH 1/2] Add type annotations to `ProductPipeline` in `build-script` This prevents possible runtime type errors and helped uncovering https://github.com/swiftlang/swift/pull/82777. It also server as an improvement o documentation, as provides inferred type annotations for users of this class and callers of its methods. --- utils/build-script | 3 +- utils/build_swift/build_swift/migration.py | 3 +- .../productpipeline_list_builder.py | 42 ++++++++++--------- .../swift_build_support/utils.py | 4 +- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/utils/build-script b/utils/build-script index d4b2ecd6d7f3c..7895744f62eab 100755 --- a/utils/build-script +++ b/utils/build-script @@ -14,6 +14,7 @@ The ultimate tool for building Swift. """ +from argparse import Namespace import json import os import platform @@ -187,7 +188,7 @@ def tar(source, destination): # ----------------------------------------------------------------------------- # Argument Validation -def validate_arguments(toolchain, args): +def validate_arguments(toolchain, args: Namespace): if toolchain.cc is None or toolchain.cxx is None: fatal_error( "can't find clang (please install clang-3.5 or a " diff --git a/utils/build_swift/build_swift/migration.py b/utils/build_swift/build_swift/migration.py index dc6e0823647b9..c2549d06cb77f 100644 --- a/utils/build_swift/build_swift/migration.py +++ b/utils/build_swift/build_swift/migration.py @@ -12,6 +12,7 @@ """ +from argparse import ArgumentParser import itertools import subprocess @@ -97,7 +98,7 @@ def _process_disambiguation_arguments(args, unknown_args): return args, unknown_args -def parse_args(parser, args, namespace=None): +def parse_args(parser: ArgumentParser, args, namespace=None): """Parses a given argument list with the given argparse.ArgumentParser. Return a processed arguments object. Any unknown arguments are stored in diff --git a/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py b/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py index 056e8af2f117f..d77a7689b7607 100644 --- a/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py +++ b/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py @@ -12,6 +12,8 @@ import platform +from typing import Optional +from products.product import Product from swift_build_support.swift_build_support import build_graph @@ -23,14 +25,14 @@ class ProductPipeline(object): This class is meant to just be state. """ - def __init__(self, should_run_epilogue_operations, identity, is_impl): + def __init__(self, should_run_epilogue_operations: bool, identity: int, is_impl: bool): assert isinstance(identity, int) self.identity = identity - self.products = [] + self.products: list[tuple[type[Product], bool]] = [] self.is_impl = is_impl self.should_run_epilogue_operations = should_run_epilogue_operations - def append(self, product, is_enabled): + def append(self, product: type[Product], is_enabled: bool): self.products.append((product, is_enabled)) def finalize(self): @@ -41,7 +43,7 @@ def finalize(self): def __iter__(self): return iter(self.products) - def __getitem__(self, i): + def __getitem__(self, i) -> tuple[type[Product], bool]: return self.products[i] def __len__(self): @@ -65,9 +67,9 @@ class ProductPipelineListBuilder(object): def __init__(self, args): self.args = args self.current_count = 0 - self.current_pipeline = None + self.current_pipeline: Optional[ProductPipeline] = None self.is_current_pipeline_impl = False - self.pipeline_list = [] + self.pipeline_list: list[ProductPipeline] = [] def begin_pipeline(self): if self.current_pipeline is not None: @@ -93,14 +95,14 @@ def reset(self): self.is_current_pipeline_impl = False self.pipelinst_list = [] - def add_product(self, product_cls, is_enabled): + def add_product(self, product_cls: type[Product], is_enabled: bool): """Add a non-impl product to the current pipeline begin constructed""" assert self.current_pipeline is not None assert not self.is_current_pipeline_impl assert not product_cls.is_build_script_impl_product() self.current_pipeline.append(product_cls, is_enabled) - def add_impl_product(self, product_cls, is_enabled): + def add_impl_product(self, product_cls: type[Product], is_enabled: bool): """Add an impl product to the current pipeline begin constructed""" assert self.current_pipeline is not None assert self.is_current_pipeline_impl @@ -109,14 +111,16 @@ def add_impl_product(self, product_cls, is_enabled): def infer(self): products_to_generation_index = {} - enabled_products = set() - inferred_pipeline_list = [] + enabled_products: set[type[Product]] = set() + inferred_pipeline_list: list[list[Optional[type[Product]]]] = [] last_impl_pipeline_index = None + for i in range(len(self.pipeline_list)): - pipeline = self.pipeline_list[i] + pipeline: ProductPipeline = self.pipeline_list[i] if pipeline.is_impl: last_impl_pipeline_index = i - final_pipeline = [] + + final_pipeline: list[Optional[type[Product]]] = [] for pipeline_i in range(len(pipeline)): (p, is_enabled) = pipeline[pipeline_i] # Make sure p has not been added multiple times to the builder. @@ -137,11 +141,11 @@ def infer(self): p.get_dependencies())) for i in range(len(inferred_pipeline_list)): - pipeline = inferred_pipeline_list[i] + inferred_pipeline = inferred_pipeline_list[i] # Filter out any of the pipelines that before inference were not # selected. - enabled_pipeline = [p for p in pipeline if p is not None] + enabled_pipeline: list = [p for p in inferred_pipeline if p is not None] if self.args.verbose_build: print("-- Build Graph Inference --") @@ -170,18 +174,18 @@ def infer(self): inferred_pipeline_list[gen_offset][index] = p filtered_results = [] - for pipeline in inferred_pipeline_list: - filtered_results.append([p for p in pipeline if p is not None]) + for inferred_pipeline in inferred_pipeline_list: + filtered_results.append([p for p in inferred_pipeline if p is not None]) if self.args.verbose_build: print("Final Build Order:") - for pipeline in filtered_results: - for p in pipeline: + for filtered_pipeline in filtered_results: + for p in filtered_pipeline: print(" {}".format(p.product_name())) return (filtered_results, last_impl_pipeline_index) - def finalize(self, shouldInfer): + def finalize(self, shouldInfer: bool): """Product a final schedule and return a list of our product pipelines. Resets the builder when done so is a consuming operation. """ diff --git a/utils/swift_build_support/swift_build_support/utils.py b/utils/swift_build_support/swift_build_support/utils.py index 8465a44da5707..3d141c9f48d0c 100644 --- a/utils/swift_build_support/swift_build_support/utils.py +++ b/utils/swift_build_support/swift_build_support/utils.py @@ -16,7 +16,7 @@ import sys import time - +from typing import NoReturn from build_swift.build_swift.constants import SWIFT_BUILD_ROOT @@ -30,7 +30,7 @@ def fatal_error(message, stream=sys.stderr): sys.exit(1) -def exit_rejecting_arguments(message, parser=None): +def exit_rejecting_arguments(message, parser=None) -> NoReturn: print(message, file=sys.stderr) if parser: parser.print_usage(sys.stderr) From 3aeffef54e36826547ef562f811a4e2341bce241 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 3 Jul 2025 22:57:38 +0100 Subject: [PATCH 2/2] Fix import path in `productpipeline_list_builder.py` --- .../swift_build_support/productpipeline_list_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py b/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py index d77a7689b7607..4d49ba5283df9 100644 --- a/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py +++ b/utils/swift_build_support/swift_build_support/productpipeline_list_builder.py @@ -13,7 +13,7 @@ import platform from typing import Optional -from products.product import Product +from .products.product import Product from swift_build_support.swift_build_support import build_graph