Skip to content

Commit 2304c47

Browse files
committed
Validate token choices in CI
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).
1 parent 9df5d69 commit 2304c47

File tree

3 files changed

+32
-18
lines changed

3 files changed

+32
-18
lines changed

Package.swift

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ import Foundation
55

66
/// If we are in a controlled CI environment, we can use internal compiler flags
77
/// to speed up the build or improve it.
8-
let swiftSyntaxSwiftSettings: [SwiftSetting]
8+
var swiftSyntaxSwiftSettings: [SwiftSetting] = []
99
if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil {
1010
let groupFile = URL(fileURLWithPath: #file)
1111
.deletingLastPathComponent()
1212
.appendingPathComponent("utils")
1313
.appendingPathComponent("group.json")
14-
swiftSyntaxSwiftSettings = [
14+
swiftSyntaxSwiftSettings += [
1515
.define("SWIFTSYNTAX_ENABLE_ASSERTIONS"),
1616
.unsafeFlags([
1717
"-Xfrontend", "-group-info-path",
@@ -21,8 +21,11 @@ if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil
2121
"-enforce-exclusivity=unchecked",
2222
]),
2323
]
24-
} else {
25-
swiftSyntaxSwiftSettings = []
24+
}
25+
if ProcessInfo.processInfo.environment["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] != nil {
26+
swiftSyntaxSwiftSettings += [
27+
.define("SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION")
28+
]
2629
}
2730

2831
let package = Package(

Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
/// Note that this only validates the immediate children.
1818
/// Results in an assertion failure if the layout is invalid.
1919
func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
20-
#if DEBUG
20+
#if SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION
2121
enum TokenChoice: CustomStringConvertible {
2222
case keyword(StaticString)
2323
case tokenKind(RawTokenKind)
@@ -140,7 +140,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
140140
// the list of expected token choices in the syntax tree doesn't match those
141141
// the parser generates. Disable the verification for now until all issues
142142
// regarding it are fixed.
143-
#if VALIDATE_TOKEN_CHOICES
144143
if raw != nil {
145144
return verify(
146145
raw,
@@ -151,9 +150,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
151150
)
152151
}
153152
return nil
154-
#else
155-
return verify(raw, as: RawTokenSyntax?.self)
156-
#endif
157153
}
158154
func verify(
159155
_ raw: RawSyntax?,
@@ -166,7 +162,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
166162
// the list of expected token choices in the syntax tree doesn't match those
167163
// the parser generates. Disable the verification for now until all issues
168164
// regarding it are fixed.
169-
#if VALIDATE_TOKEN_CHOICES
170165
guard let raw = raw else {
171166
return .expectedNonNil(expectedKind: RawTokenSyntax.self, file: file, line: line)
172167
}
@@ -193,9 +188,6 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
193188
file: file,
194189
line: line
195190
)
196-
#else
197-
return verify(raw, as: RawTokenSyntax.self)
198-
#endif
199191
}
200192
func assertNoError(_ nodeKind: SyntaxKind, _ index: Int, _ error: ValidationError?) {
201193
if let error = error {

build-script.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import subprocess
77
import sys
88
import tempfile
9-
from concurrent.futures import ThreadPoolExecutor
109
from typing import Dict, List, Optional
1110

1211

@@ -141,6 +140,7 @@ def run_code_generation(
141140

142141
env = dict(os.environ)
143142
env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1"
143+
env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1"
144144
check_call(swiftpm_call, env=env, verbose=verbose)
145145

146146

@@ -174,6 +174,7 @@ class Builder(object):
174174
build_dir: Optional[str]
175175
multiroot_data_file: Optional[str]
176176
release: bool
177+
enable_rawsyntax_validation: bool
177178
disable_sandbox: bool
178179

179180
def __init__(
@@ -182,12 +183,14 @@ def __init__(
182183
build_dir: Optional[str],
183184
multiroot_data_file: Optional[str],
184185
release: bool,
186+
enable_rawsyntax_validation: bool,
185187
verbose: bool,
186188
disable_sandbox: bool = False,
187189
) -> None:
188190
self.build_dir = build_dir
189191
self.multiroot_data_file = multiroot_data_file
190192
self.release = release
193+
self.enable_rawsyntax_validation = enable_rawsyntax_validation
191194
self.disable_sandbox = disable_sandbox
192195
self.verbose = verbose
193196
self.toolchain = toolchain
@@ -221,6 +224,8 @@ def __build(self, package_dir: str, product_name: str) -> None:
221224

222225
env = dict(os.environ)
223226
env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1"
227+
if self.enable_rawsyntax_validation:
228+
env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1"
224229
# Tell other projects in the unified build to use local dependencies
225230
env["SWIFTCI_USE_LOCAL_DEPS"] = "1"
226231
env["SWIFT_SYNTAX_PARSER_LIB_SEARCH_PATH"] = \
@@ -274,7 +279,8 @@ def check_generated_files_match(self_generated_dir: str,
274279

275280
def run_tests(
276281
toolchain: str, build_dir: Optional[str], multiroot_data_file: Optional[str],
277-
release: bool, filecheck_exec: Optional[str], skip_lit_tests: bool, verbose: bool
282+
release: bool, enable_rawsyntax_validation: bool, filecheck_exec: Optional[str],
283+
skip_lit_tests: bool, verbose: bool
278284
) -> None:
279285
print("** Running SwiftSyntax Tests **")
280286

@@ -292,6 +298,7 @@ def run_tests(
292298
build_dir=build_dir,
293299
multiroot_data_file=multiroot_data_file,
294300
release=release,
301+
enable_rawsyntax_validation=enable_rawsyntax_validation,
295302
verbose=verbose,
296303
)
297304

@@ -396,6 +403,7 @@ def run_lit_tests(toolchain: str, build_dir: Optional[str], release: bool,
396403

397404
def run_xctests(toolchain: str, build_dir: Optional[str],
398405
multiroot_data_file: Optional[str], release: bool,
406+
enable_rawsyntax_validation: bool,
399407
verbose: bool) -> None:
400408
print("** Running XCTests **")
401409
swiftpm_call = get_swiftpm_invocation(
@@ -414,6 +422,8 @@ def run_xctests(toolchain: str, build_dir: Optional[str],
414422

415423
env = dict(os.environ)
416424
env["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] = "1"
425+
if enable_rawsyntax_validation:
426+
env["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] = "1"
417427
# Tell other projects in the unified build to use local dependencies
418428
env["SWIFTCI_USE_LOCAL_DEPS"] = "1"
419429
env["SWIFT_SYNTAX_PARSER_LIB_SEARCH_PATH"] = \
@@ -461,7 +471,7 @@ def verify_source_code_command(args: argparse.Namespace) -> None:
461471
def build_command(args: argparse.Namespace) -> None:
462472
try:
463473
builder = Builder(
464-
toolchain=realpath(args.toolchain),
474+
toolchain=realpath(args.toolchain), # pyright: ignore
465475
build_dir=realpath(args.build_dir),
466476
multiroot_data_file=args.multiroot_data_file,
467477
release=args.release,
@@ -484,7 +494,7 @@ def build_command(args: argparse.Namespace) -> None:
484494
def test_command(args: argparse.Namespace) -> None:
485495
try:
486496
builder = Builder(
487-
toolchain=realpath(args.toolchain),
497+
toolchain=realpath(args.toolchain), # pyright: ignore
488498
build_dir=realpath(args.build_dir),
489499
multiroot_data_file=args.multiroot_data_file,
490500
release=args.release,
@@ -496,7 +506,7 @@ def test_command(args: argparse.Namespace) -> None:
496506
builder.buildExample("ExamplePlugin")
497507

498508
run_tests(
499-
toolchain=realpath(args.toolchain),
509+
toolchain=realpath(args.toolchain), # pyright: ignore
500510
build_dir=realpath(args.build_dir),
501511
multiroot_data_file=args.multiroot_data_file,
502512
release=args.release,
@@ -552,6 +562,15 @@ def add_default_build_arguments(parser: argparse.ArgumentParser) -> None:
552562
""",
553563
)
554564

565+
parser.add_argument(
566+
"--enable-rawsyntax-validation",
567+
help="""
568+
When constructing RawSyntax nodes validate that their layout matches that
569+
defined in `CodeGeneration` and that TokenSyntax nodes have a `tokenKind`
570+
matching the ones specified in `CodeGeneration`.
571+
"""
572+
)
573+
555574
parser.add_argument(
556575
"--toolchain",
557576
required=True,

0 commit comments

Comments
 (0)