Skip to content

Commit a29be0b

Browse files
authored
clean up wordy comments across macOS bring-up changes
1 parent de5c1f7 commit a29be0b

8 files changed

Lines changed: 110 additions & 262 deletions

File tree

‎.bazelrc‎

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,40 +12,14 @@ common --override_module=semmle_code=%workspace%/misc/bazel/semmle_code_stub
1212

1313
build --repo_env=CC=clang --repo_env=CXX=clang++
1414

15-
# Point Bazel at `apple_support`'s xcode_config on macOS.
16-
#
17-
# `xcode_swift_toolchain` pulls in `@system_sdk//:{all_modules,implicit_modules}`,
18-
# whose `select({":xcode_<version>": ...})` keys use the exact version string
19-
# from `@bazel_tools//tools/osx:xcode_locator.m` (e.g. `xcode_26_6_0_17F113`).
20-
# Bazel's built-in `@bazel_tools//tools/osx:host_xcodes` produces a shorter
21-
# version without the build number, so none of the config_settings match and
22-
# the select fails.
23-
#
24-
# `apple_support`'s `@local_config_xcode//:host_xcodes` uses the SAME
25-
# `xcode_locator.m`, so its version string agrees with the select keys.
26-
# Wired up in `MODULE.bazel` via the `xcode_configure` extension.
15+
# Use apple_support's xcode_config: its version strings match the ones
16+
# `rules_swift`'s `system_sdk` selects are keyed on.
2717
build:macos --xcode_version_config=@local_config_xcode//:host_xcodes
2818

29-
# Give `apple_support`'s CC toolchains priority over Bazel's built-in
30-
# `@rules_cc//...:local_config_cc` on macOS.
31-
#
32-
# `apple_support` registers `@local_config_apple_cc_toolchains//:all` from
33-
# its own `MODULE.bazel`, but module-registered toolchains are ordered by
34-
# the module DAG and Bazel's built-in `local_config_cc` (pulled in by
35-
# `rules_cc`) wins resolution first. That toolchain reports a
36-
# `target_gnu_system_name` of literally `"local"`, and
37-
# `rules_swift`'s `xcode_swift_toolchain` blows up in
38-
# `target_triples.parse(ctx.var.get("CC_TARGET_TRIPLE") or
39-
# cc_toolchain.target_gnu_system_name)` with:
40-
#
41-
# Invalid target triple: local, this likely means you're using the wrong
42-
# CC toolchain, make sure you include apple_support in your project
43-
#
44-
# Root-level `--extra_toolchains` beats all module-registered toolchains,
45-
# so this forces `@local_config_apple_cc_toolchains//:cc-toolchain-darwin_*`
46-
# to be selected for both the target and exec platforms. The apple_cc
47-
# toolchains only match Apple platform constraints, so this is a no-op
48-
# for any non-Apple exec/target platform.
19+
# Force apple_support's CC toolchains over Bazel's built-in `local_config_cc`
20+
# (whose `target_gnu_system_name` is literally "local", which breaks
21+
# `xcode_swift_toolchain`'s triple parsing). Root-level `--extra_toolchains`
22+
# beats module-registered toolchains; only matches Apple platforms.
4923
build:macos --extra_toolchains=@local_config_apple_cc_toolchains//:all
5024

5125
# Disable Android SDK auto-detection (we don't use it, and rules_android has Bazel 9 compatibility issues)

‎MODULE.bazel‎

Lines changed: 16 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,9 @@ bazel_dep(name = "googletest", version = "1.17.0.bcr.2")
3333
bazel_dep(name = "rules_rust", version = "0.69.0")
3434
bazel_dep(name = "rules_swift", version = "4.0.0-rc4")
3535
bazel_dep(name = "swift-syntax", version = "602.0.0.bcr.2")
36-
# `apple_support` is a transitive dep of `rules_swift`, but we depend on it
37-
# directly for two reasons on macOS:
38-
#
39-
# 1. We `use_repo` `local_config_xcode` (see below) so
40-
# `xcode_swift_toolchain`'s `system_sdk` selects — which are keyed on
41-
# the exact Xcode version string from `xcode_locator.m` (e.g.
42-
# `xcode_26_6_0_17F113`) — resolve.
43-
#
44-
# 2. It registers `@local_config_apple_cc_toolchains//:all`, whose
45-
# `cc-toolchain-darwin_*` toolchains report a proper Apple target
46-
# triple (`arm64-apple-macosx` / `x86_64-apple-macosx`) via
47-
# `target_gnu_system_name`. Bazel's built-in `local_config_cc`
48-
# toolchain reports the literal string `"local"`, and
49-
# `xcode_swift_toolchain` fails analysis in
50-
# `target_triples.parse(ctx.var.get("CC_TARGET_TRIPLE") or
51-
# cc_toolchain.target_gnu_system_name)` with
52-
# "Invalid target triple: local". Since module-registered toolchains
53-
# are ordered by the module DAG and `local_config_cc` wins by default,
54-
# we override the priority via `--extra_toolchains` in `.bazelrc`.
36+
# Direct dep on `apple_support` (transitively pulled in by `rules_swift`) so we
37+
# can `use_repo` `local_config_xcode` and `local_config_apple_cc_toolchains`
38+
# below, needed to make `xcode_swift_toolchain` work on macOS. See `.bazelrc`.
5539
bazel_dep(name = "apple_support", version = "2.6.1")
5640

5741
bazel_dep(name = "zstd", version = "1.5.7.bcr.1")
@@ -242,39 +226,12 @@ use_repo(
242226
"swift-resource-dir-macos",
243227
)
244228

245-
# Hermetic Swift toolchain (from swift.org) for building the `swift-syntax`
246-
# based Rust wrapper in `unified/swift-syntax-rs`. This is the official
247-
# `rules_swift` standalone-toolchain extension and is independent of the
248-
# patched prebuilt toolchain wired up via `swift_deps` above.
249-
#
250-
# We register the `exec` toolchain for **Linux only**. On macOS we do NOT
251-
# use this standalone toolchain: it is the Unix-shaped `swift_toolchain`
252-
# rule (`swift/toolchains/swift_toolchain.bzl`) which emits Linux/ELF
253-
# linkopts (`swiftrt.o`, `-lstdc++`, `-lrt`, `-ldl`, `-static-libgcc`) and
254-
# a Linux-shaped `-L{root}/lib/swift/{os}` search path — none of which make
255-
# sense on macOS. Instead, on macOS we rely on rules_swift's auto-registered
256-
# `@rules_swift//swift/toolchains:xcode-toolchain-*` toolchains (backed by
257-
# `xcode_swift_toolchain.bzl`, which has its own macOS-aware
258-
# `_swift_linkopts_cc_info` that uses the Apple linker's autolinking,
259-
# points at `Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx`,
260-
# and rpaths the OS-provided `/usr/lib/swift`). Those toolchains are
261-
# auto-registered by rules_swift's own MODULE.bazel via
262-
# `register_toolchains("//swift/toolchains:all")` and use the Xcode
263-
# selected on the host, so macOS contributors need a working Xcode
264-
# (or Command Line Tools) install.
265-
#
266-
# NOTE: The macOS standalone toolchain is distributed as a `.pkg` archive
267-
# that can only be extracted on a macOS host. Do not build the `xcode`
268-
# standalone toolchain on Linux.
269-
#
270-
# We register the `exec` toolchain (normal host compilation) rather than the
271-
# `embedded` one, which targets `os:none` (Embedded Swift) and would not
272-
# match a normal host build. Bazel selects the toolchain matching the host
273-
# platform automatically.
274-
#
275-
# The Swift version is read from `unified/swift-syntax-rs/.swift-version`, which
276-
# is the single source of truth shared with the local `cargo` build (via
277-
# swiftly / any Swift install) and `swift/Package.swift`.
229+
# Hermetic Swift toolchain (swift.org) for building `unified/swift-syntax-rs`.
230+
# Only registered as an `exec` toolchain on Linux; on macOS we rely on
231+
# `rules_swift`'s auto-registered `xcode_swift_toolchain`, which uses the
232+
# host Xcode and links against the OS-provided Swift runtime in `/usr/lib/swift`.
233+
# Version is read from `unified/swift-syntax-rs/.swift-version`, shared with
234+
# the local `cargo` build and `swift/Package.swift`.
278235
swift = use_extension("@rules_swift//swift:extensions.bzl", "swift")
279236
swift.toolchain(
280237
name = "swift_toolchain",
@@ -290,29 +247,16 @@ register_toolchains(
290247
"@swift_toolchain//:swift_toolchain_exec_ubuntu24.04",
291248
)
292249

293-
# On macOS, `xcode_swift_toolchain` (auto-registered by rules_swift at
294-
# `@rules_swift//swift/toolchains:xcode-toolchain-*`) pulls in
295-
# `@system_sdk//:{all_modules,implicit_modules}`, whose targets are
296-
# `alias(..., select({":xcode_<version>": ...}))` keyed on the exact
297-
# Xcode version string produced by `@bazel_tools//tools/osx:xcode_locator.m`
298-
# (e.g. `xcode_26_6_0_17F113`). Bazel's built-in `@bazel_tools//tools/osx:host_xcodes`
299-
# reports a shorter version without the build number, so no `config_setting`
300-
# matches and the select fails with
301-
# "configurable attribute doesn't match this configuration".
302-
#
303-
# `apple_support`'s `@local_config_xcode//:host_xcodes` uses the SAME
304-
# `xcode_locator.m` as rules_swift's `system_sdk` extension, so its version
305-
# strings agree with the config_setting keys. Wire it up here and select it
306-
# on macOS via `--xcode_version_config` in `.bazelrc`.
250+
# `apple_support`'s xcode_config, selected via `--xcode_version_config` in
251+
# `.bazelrc`. Needed because rules_swift's `system_sdk` selects use Xcode
252+
# version strings from `apple_support`'s `xcode_locator.m`, which Bazel's
253+
# built-in `host_xcodes` does not match.
307254
xcode_configure = use_extension("@apple_support//xcode:xcode_configure.bzl", "xcode_configure_extension")
308255
use_repo(xcode_configure, "local_config_xcode")
309256

310-
# `apple_support` also registers `@local_config_apple_cc_toolchains//:all`
311-
# from its own `MODULE.bazel`, but the CC toolchains it generates lose
312-
# toolchain-resolution ordering to Bazel's built-in `local_config_cc`.
313-
# We override that priority in `.bazelrc` via
314-
# `build:macos --extra_toolchains=@local_config_apple_cc_toolchains//:all`,
315-
# which requires the repo to be visible from the root module.
257+
# `apple_support`'s CC toolchains, forced ahead of Bazel's built-in
258+
# `local_config_cc` via `--extra_toolchains` in `.bazelrc`. Must be visible
259+
# from the root module for the flag to resolve.
316260
apple_cc_configure = use_extension("@apple_support//crosstool:setup.bzl", "apple_cc_configure_extension")
317261
use_repo(apple_cc_configure, "local_config_apple_cc_toolchains")
318262

‎unified/extractor/src/languages/swift/adapter.rs‎

Lines changed: 33 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,19 @@
1-
//! Converts the swift-syntax JSON syntax tree into a [`yeast::Ast`], the
2-
//! in-memory format the CodeQL desugaring rules operate on.
1+
//! Converts the swift-syntax JSON syntax tree into a [`yeast::Ast`].
32
//!
4-
//! The JSON tree is produced by the `swift-syntax-rs` crate's Swift FFI shim
5-
//! (`parse_to_json`). This module is pure Rust (only `yeast` + `serde_json`),
6-
//! so the extractor consumes swift-syntax output without pulling in the Swift
7-
//! toolchain (the JSON is produced out-of-process).
3+
//! The JSON is produced out-of-process by `swift-syntax-rs`'s FFI shim, so
4+
//! this module is pure Rust (no Swift toolchain needed).
85
//!
9-
//! The mapping mirrors tree-sitter's node model, which is what yeast (and the
10-
//! extractor's rewrite rules) expect:
6+
//! The mapping mirrors tree-sitter's node model:
117
//!
12-
//! * **Layout nodes** (e.g. `functionDecl`) and **varying tokens** (identifiers,
13-
//! literals, operators — the ones whose text is not determined by their kind)
14-
//! become **named** nodes, keyed by their kind name.
15-
//! * **Fixed tokens** (keywords and punctuation, whose text is fully determined
16-
//! by their kind) become **anonymous** nodes, keyed by their text — exactly
17-
//! how tree-sitter models anonymous tokens (e.g. `"func"`, `"->"`).
18-
//! * Collection nodes are already elided to JSON arrays upstream, so a
19-
//! list-valued field maps directly to that field holding several children.
8+
//! * **Layout nodes** (e.g. `functionDecl`) and **varying tokens**
9+
//! (identifiers, literals, operators — text not determined by kind) become
10+
//! **named** nodes, keyed by their kind name.
11+
//! * **Fixed tokens** (keywords, punctuation) become **anonymous** nodes,
12+
//! keyed by their text (like tree-sitter's anonymous tokens).
13+
//! * Collection nodes are already elided to JSON arrays upstream.
2014
//!
21-
//! Note: this preserves swift-syntax's own kind/field names. Aligning those
22-
//! names with the tree-sitter-swift schema (so the rewrite rules in
23-
//! [`super::swift`] fire) is done incrementally in the rules.
15+
//! Kind/field names are preserved from swift-syntax; aligning them with the
16+
//! tree-sitter-swift schema is done incrementally in the rewrite rules.
2417
2518
use std::collections::BTreeMap;
2619

@@ -29,32 +22,29 @@ use yeast::schema::Schema;
2922
use yeast::{Ast, Id, NodeContent, Point, Range};
3023

3124
/// A comment (or `unexpectedText`) recovered from the syntax tree's trivia.
32-
///
33-
/// These are collected into a side channel rather than embedded in the
34-
/// [`yeast::Ast`], mirroring how the extractor treats tree-sitter `extra`
35-
/// nodes: they carry a location and text but are not attached to a parent.
25+
/// Collected into a side channel rather than embedded in the [`yeast::Ast`],
26+
/// mirroring how the extractor treats tree-sitter `extra` nodes.
3627
#[derive(Debug, Clone, PartialEq, Eq)]
3728
pub struct TriviaToken {
38-
/// The trivia kind (e.g. `lineComment`, `blockComment`, `docLineComment`,
39-
/// `docBlockComment`, `unexpectedText`).
29+
/// The trivia kind (e.g. `lineComment`, `docLineComment`, `unexpectedText`).
4030
pub kind: String,
41-
/// The verbatim source text of the piece (e.g. `// comment`).
31+
/// The verbatim source text (e.g. `// comment`).
4232
pub text: String,
4333
/// The source range the piece occupies.
4434
pub range: Range,
4535
}
4636

47-
/// The result of adapting a swift-syntax JSON tree: the [`yeast::Ast`] plus the
48-
/// comment/`unexpectedText` trivia harvested from it (in source order).
37+
/// The result of adapting a swift-syntax JSON tree: the [`yeast::Ast`] plus
38+
/// the comment/`unexpectedText` trivia harvested from it (in source order).
4939
pub struct AdaptedTree {
5040
pub ast: Ast,
5141
pub trivia: Vec<TriviaToken>,
5242
}
5343

5444
/// swift-syntax `TokenKind` cases whose text is *not* determined by the kind
55-
/// (i.e. `TokenKind.defaultText == nil`). These carry varying information and
56-
/// are modelled as named leaf nodes; every other token is a fixed
57-
/// keyword/punctuation token modelled as an anonymous token keyed by its text.
45+
/// (i.e. `TokenKind.defaultText == nil`). Modelled as named leaf nodes; every
46+
/// other token is a fixed keyword/punctuation, modelled as an anonymous token
47+
/// keyed by its text.
5848
const VARYING_TOKEN_KINDS: &[&str] = &[
5949
"identifier",
6050
"integerLiteral",
@@ -116,7 +106,7 @@ fn classify(node: &Value) -> Result<KindInfo, String> {
116106
.unwrap_or("")
117107
.to_string();
118108

119-
// The case name is the part before any `(payload)` in the debug rendering.
109+
// Case name is the part before any `(payload)` in the debug rendering.
120110
let case_name = token_kind.split('(').next().unwrap_or(token_kind);
121111

122112
if VARYING_TOKEN_KINDS.contains(&case_name) {
@@ -142,8 +132,7 @@ fn classify(node: &Value) -> Result<KindInfo, String> {
142132
}
143133
}
144134

145-
/// Iterate over a node object's structural (field, value) pairs in a stable
146-
/// order, skipping metadata keys.
135+
/// Iterate over a node's structural (field, value) pairs, skipping metadata.
147136
fn field_entries(node: &Value) -> Vec<(&str, &Value)> {
148137
node.as_object()
149138
.map(|map| {
@@ -155,8 +144,8 @@ fn field_entries(node: &Value) -> Vec<(&str, &Value)> {
155144
.unwrap_or_default()
156145
}
157146

158-
/// The child node objects held by a field value, which is either a single node
159-
/// object or an array of them (an elided collection).
147+
/// Child nodes held by a field value: either a single node or an elided
148+
/// collection (JSON array).
160149
fn children_of(value: &Value) -> Vec<&Value> {
161150
match value {
162151
Value::Array(items) => items.iter().collect(),
@@ -165,12 +154,8 @@ fn children_of(value: &Value) -> Vec<&Value> {
165154
}
166155

167156
/// Recursively build `node` (and its descendants) into `ast`, returning its id.
168-
///
169-
/// This is a single traversal: each node's kind and field names are registered
170-
/// in the schema on the fly, immediately before the node is created. Children
171-
/// are built first so a parent's field lists reference existing ids. Any
172-
/// comment/`unexpectedText` trivia carried by a token is harvested into
173-
/// `trivia` during the same pass rather than embedded in the tree.
157+
/// Kinds and field names are registered in the schema on the fly. Comment /
158+
/// `unexpectedText` trivia is harvested into `trivia` in the same pass.
174159
fn build(node: &Value, ast: &mut Ast, trivia: &mut Vec<TriviaToken>) -> Result<Id, String> {
175160
let info = classify(node)?;
176161
collect_trivia(node, trivia);
@@ -200,9 +185,8 @@ fn build(node: &Value, ast: &mut Ast, trivia: &mut Vec<TriviaToken>) -> Result<I
200185
))
201186
}
202187

203-
/// Harvest a token's `leadingTrivia`/`trailingTrivia` pieces (each already
204-
/// filtered to comments/`unexpectedText` upstream) into `out`. Non-token nodes
205-
/// have no trivia keys, so this is a no-op for them.
188+
/// Harvest a token's `leadingTrivia`/`trailingTrivia` pieces (already filtered
189+
/// upstream) into `out`. No-op on non-token nodes.
206190
fn collect_trivia(node: &Value, out: &mut Vec<TriviaToken>) {
207191
for key in ["leadingTrivia", "trailingTrivia"] {
208192
let Some(Value::Array(pieces)) = node.get(key) else {
@@ -231,11 +215,9 @@ fn collect_trivia(node: &Value, out: &mut Vec<TriviaToken>) {
231215

232216
/// Parse a node's `range` into a [`yeast::Range`].
233217
///
234-
/// The JSON carries, for `start` and `end`, a 0-based UTF-8 file byte `offset`,
235-
/// a 1-based `line`, and a 1-based UTF-8 byte `column`. yeast (like tree-sitter)
236-
/// uses byte offsets with 0-based rows/columns and an exclusive end, so the
237-
/// line/column are shifted down by one. swift-syntax's end position is already
238-
/// exclusive, so the byte offsets map across directly.
218+
/// JSON offsets are 0-based UTF-8 byte offsets; `line`/`column` are 1-based
219+
/// UTF-8. yeast uses byte offsets with 0-based rows/columns (exclusive end),
220+
/// so line/column are shifted down by one.
239221
fn parse_range(node: &Value) -> Option<Range> {
240222
let range = node.get("range")?;
241223
let point = |key: &str| -> Option<(usize, Point)> {
@@ -259,8 +241,7 @@ fn parse_range(node: &Value) -> Option<Range> {
259241
}
260242

261243
/// Convert a swift-syntax JSON tree (as produced by [`crate::parse_to_json`])
262-
/// into a [`yeast::Ast`] plus the comment/`unexpectedText` trivia harvested
263-
/// from it. Both are produced in a single traversal.
244+
/// into a [`yeast::Ast`] plus harvested trivia, in a single traversal.
264245
pub fn json_to_ast(json: &str) -> Result<AdaptedTree, String> {
265246
let root: Value = serde_json::from_str(json).map_err(|e| format!("invalid JSON: {e}"))?;
266247

‎unified/extractor/src/languages/swift/swift.rs‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,12 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
135135
// Declarations may be wrapped in local/global wrapper nodes.
136136
rule!((global_declaration _ @inner) => stmt { inner }),
137137
rule!((local_declaration _ @inner) => stmt { inner }),
138-
// ---- swift-syntax front-end (minimal hook-up) ----
139-
// These rules target the swift-syntax AST (camelCase kind names),
140-
// produced by the sibling `adapter` module. They coexist with the
141-
// tree-sitter rules (snake_case names): rules are dispatched by exact
142-
// kind name, and the two name spaces never collide, so these are inert
143-
// on the tree-sitter path. Only the minimal top-level mapping lives here
144-
// to demonstrate the pipeline end-to-end; the full translation is added
145-
// separately. Unmatched swift-syntax nodes fall through to the
146-
// `unsupported_node` fallback at the end.
147-
//
148-
// `sourceFile` holds its top-level statements in an (elided)
149-
// `statements` collection; each element is a `codeBlockItem` wrapping
150-
// the real node.
138+
// ---- swift-syntax front-end ----
139+
// Rules targeting the swift-syntax AST (camelCase kind names, built
140+
// by the sibling `adapter` module). Kind namespaces don't overlap
141+
// with the tree-sitter (snake_case) rules, so these are inert on
142+
// the tree-sitter path. Unmatched nodes fall through to the
143+
// `unsupported_node` fallback below.
151144
rule!(
152145
(sourceFile statements: _* @items)
153146
=>

0 commit comments

Comments
 (0)