From e725602221a36fc1a5df20f5c48b98d07959f871 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 29 Mar 2023 10:14:17 -0700 Subject: [PATCH] Validate token choices in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the the paradigm around token validation in SwiftSyntax. Previously, we always validated that the `RawSyntax` tree layout was as expected in debug builds (independent of whether we forced assertions on). This property should always be guaranteed by our API unless you are replacing children manually. Verification that the tokens in the tree matched the token choices, on the other hand, was always disabled unless you specifically enabled it by modifying Package.swift. This changes the paradigm by enabling `RawSyntax` layout and token validation whenever the `SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION` conditional compilation flag is set. I am planning to set this compilation flag on SwiftSyntax’s PR testing jobs at first and, after considering the performance impact, also on Swift’s PR jobs. This should give us two advantages: - In CI we get more extensive tree validation because we are now also validate the token choices - If we are just building SwiftSyntax as a package dependency in SwiftPM, we get faster performance because we skip the `RawSyntax` layout validation (which is really just a way to debug serious issues while working on SwiftSyntax itself). --- Package.swift | 11 ++++--- .../generated/raw/RawSyntaxValidation.swift | 10 +----- build-script.py | 33 ++++++++++++++++--- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Package.swift b/Package.swift index f22a197767f..6d26c21167c 100644 --- a/Package.swift +++ b/Package.swift @@ -5,13 +5,13 @@ import Foundation /// If we are in a controlled CI environment, we can use internal compiler flags /// to speed up the build or improve it. -let swiftSyntaxSwiftSettings: [SwiftSetting] +var swiftSyntaxSwiftSettings: [SwiftSetting] = [] if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil { let groupFile = URL(fileURLWithPath: #file) .deletingLastPathComponent() .appendingPathComponent("utils") .appendingPathComponent("group.json") - swiftSyntaxSwiftSettings = [ + swiftSyntaxSwiftSettings += [ .define("SWIFTSYNTAX_ENABLE_ASSERTIONS"), .unsafeFlags([ "-Xfrontend", "-group-info-path", @@ -21,8 +21,11 @@ if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil "-enforce-exclusivity=unchecked", ]), ] -} else { - swiftSyntaxSwiftSettings = [] +} +if ProcessInfo.processInfo.environment["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] != nil { + swiftSyntaxSwiftSettings += [ + .define("SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION") + ] } let package = Package( diff --git a/Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift b/Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift index f1cb23c3a2a..a8a31b2d37f 100644 --- a/Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift +++ b/Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift @@ -17,7 +17,7 @@ /// Note that this only validates the immediate children. /// Results in an assertion failure if the layout is invalid. func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) { - #if DEBUG + #if SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION enum TokenChoice: CustomStringConvertible { case keyword(StaticString) case tokenKind(RawTokenKind) @@ -140,7 +140,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) { // the list of expected token choices in the syntax tree doesn't match those // the parser generates. Disable the verification for now until all issues // regarding it are fixed. - #if VALIDATE_TOKEN_CHOICES if raw != nil { return verify( raw, @@ -151,9 +150,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) { ) } return nil - #else - return verify(raw, as: RawTokenSyntax?.self) - #endif } func verify( _ raw: RawSyntax?, @@ -166,7 +162,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) { // the list of expected token choices in the syntax tree doesn't match those // the parser generates. Disable the verification for now until all issues // regarding it are fixed. - #if VALIDATE_TOKEN_CHOICES guard let raw = raw else { return .expectedNonNil(expectedKind: RawTokenSyntax.self, file: file, line: line) } @@ -193,9 +188,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) { file: file, line: line ) - #else - return verify(raw, as: RawTokenSyntax.self) - #endif } func assertNoError(_ nodeKind: SyntaxKind, _ index: Int, _ error: ValidationError?) { if let error = error { diff --git a/build-script.py b/build-script.py index f4a742223c1..7e8daa5549d 100755 --- a/build-script.py +++ b/build-script.py @@ -6,7 +6,6 @@ import subprocess import sys import tempfile -from concurrent.futures import ThreadPoolExecutor from typing import Dict, List, Optional @@ -141,6 +140,7 @@ def run_code_generation( env = dict(os.environ) env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1" + env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1" check_call(swiftpm_call, env=env, verbose=verbose) @@ -174,6 +174,7 @@ class Builder(object): build_dir: Optional[str] multiroot_data_file: Optional[str] release: bool + enable_rawsyntax_validation: bool disable_sandbox: bool def __init__( @@ -182,12 +183,14 @@ def __init__( build_dir: Optional[str], multiroot_data_file: Optional[str], release: bool, + enable_rawsyntax_validation: bool, verbose: bool, disable_sandbox: bool = False, ) -> None: self.build_dir = build_dir self.multiroot_data_file = multiroot_data_file self.release = release + self.enable_rawsyntax_validation = enable_rawsyntax_validation self.disable_sandbox = disable_sandbox self.verbose = verbose self.toolchain = toolchain @@ -221,6 +224,8 @@ def __build(self, package_dir: str, product_name: str) -> None: env = dict(os.environ) env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1" + if self.enable_rawsyntax_validation: + env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1" # Tell other projects in the unified build to use local dependencies env["SWIFTCI_USE_LOCAL_DEPS"] = "1" env["SWIFT_SYNTAX_PARSER_LIB_SEARCH_PATH"] = \ @@ -274,7 +279,8 @@ def check_generated_files_match(self_generated_dir: str, def run_tests( toolchain: str, build_dir: Optional[str], multiroot_data_file: Optional[str], - release: bool, filecheck_exec: Optional[str], skip_lit_tests: bool, verbose: bool + release: bool, enable_rawsyntax_validation: bool, filecheck_exec: Optional[str], + skip_lit_tests: bool, verbose: bool ) -> None: print("** Running SwiftSyntax Tests **") @@ -292,6 +298,7 @@ def run_tests( build_dir=build_dir, multiroot_data_file=multiroot_data_file, release=release, + enable_rawsyntax_validation=enable_rawsyntax_validation, verbose=verbose, ) @@ -396,6 +403,7 @@ def run_lit_tests(toolchain: str, build_dir: Optional[str], release: bool, def run_xctests(toolchain: str, build_dir: Optional[str], multiroot_data_file: Optional[str], release: bool, + enable_rawsyntax_validation: bool, verbose: bool) -> None: print("** Running XCTests **") swiftpm_call = get_swiftpm_invocation( @@ -414,6 +422,8 @@ def run_xctests(toolchain: str, build_dir: Optional[str], env = dict(os.environ) env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1" + if enable_rawsyntax_validation: + env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1" # Tell other projects in the unified build to use local dependencies env["SWIFTCI_USE_LOCAL_DEPS"] = "1" env["SWIFT_SYNTAX_PARSER_LIB_SEARCH_PATH"] = \ @@ -461,10 +471,11 @@ def verify_source_code_command(args: argparse.Namespace) -> None: def build_command(args: argparse.Namespace) -> None: try: builder = Builder( - toolchain=realpath(args.toolchain), + toolchain=realpath(args.toolchain), # pyright: ignore build_dir=realpath(args.build_dir), multiroot_data_file=args.multiroot_data_file, release=args.release, + enable_rawsyntax_validation=args.enable_rawsyntax_validation, verbose=args.verbose, disable_sandbox=args.disable_sandbox, ) @@ -484,10 +495,11 @@ def build_command(args: argparse.Namespace) -> None: def test_command(args: argparse.Namespace) -> None: try: builder = Builder( - toolchain=realpath(args.toolchain), + toolchain=realpath(args.toolchain), # pyright: ignore build_dir=realpath(args.build_dir), multiroot_data_file=args.multiroot_data_file, release=args.release, + enable_rawsyntax_validation=args.enable_rawsyntax_validation, verbose=args.verbose, disable_sandbox=args.disable_sandbox, ) @@ -496,10 +508,11 @@ def test_command(args: argparse.Namespace) -> None: builder.buildExample("ExamplePlugin") run_tests( - toolchain=realpath(args.toolchain), + toolchain=realpath(args.toolchain), # pyright: ignore build_dir=realpath(args.build_dir), multiroot_data_file=args.multiroot_data_file, release=args.release, + enable_rawsyntax_validation=args.enable_rawsyntax_validation, filecheck_exec=realpath(args.filecheck_exec), skip_lit_tests=args.skip_lit_tests, verbose=args.verbose, @@ -552,6 +565,16 @@ def add_default_build_arguments(parser: argparse.ArgumentParser) -> None: """, ) + parser.add_argument( + "--enable-rawsyntax-validation", + action="store_true", + help=""" + When constructing RawSyntax nodes validate that their layout matches that + defined in `CodeGeneration` and that TokenSyntax nodes have a `tokenKind` + matching the ones specified in `CodeGeneration`. + """ + ) + parser.add_argument( "--toolchain", required=True,