[nfc] Add IWYU guidelines to AGENTS.md, Remove rust_io_link_check

.opencode/skills/iwyu/SKILL.md

@@ -0,0 +1,30 @@
+---
+name: iwyu
+description: Follow IWYU guidelines to improve build caching. This entails adding/removing C++
+includes as well as Bazel deps and implementation_deps when appropriate. Load when making non-
+trivial changes to C++ includes or Bazel target declarations.
+---
+
+## IWYU FOR C++ TARGETS
+
+Adapted from [include-what-you-use](https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md).
+Small targets with accurate deps mean a change rebuilds and retests fewer targets.
+
+- Don't add `#include`s a file doesn't use or `deps` a target doesn't use. When a change removes
+  the last use of a header, delete its `#include`, then the dep if it's now unused.
+- Put deps whose headers are included only from `.c++` files in `implementation_deps`, so they
+  don't propagate to dependents.
+- Prefer direct includes and deps over relying on transitive ones, but don't add a long list just
+  to spell out what you already get transitively. For instance, most workerd C++ sources depend on
+  KJ but we don't declare this dependency for each target separately, or include kj/common.h,
+  kj/string.h and so on each time. Direct includes that are already present and being used should
+  generally stay in place.
+- Give each `.h`/`.c++` pair its own `wd_cc_library`, named after the file (e.g.
+  `:tracked-wasm-instance`). A header without a `.c++` gets its own target too, with the deps it
+  needs to compile on its own.
+- Keep each header in exactly one target, together with its `.c++` if it has one. Exposing a
+  header from a target that doesn't build its `.c++` lets code call its functions without
+  depending on their definitions, which shows up later as undefined-symbol link errors.
+- If splitting a pair out would create a dependency cycle, leave it in its combined target (e.g.
+  `//src/workerd/io`) instead of separating the header from its `.c++`. Don't add new files to
+  such targets if they can have their own.

build/fixtures/BUILD.bazel

@@ -1,4 +1 @@
-exports_files([
-    "kj_test.sh",
-    "rust_io_link_check.sh",
-])
+exports_files(["kj_test.sh"])

build/rust_io_graph_check.sh

@@ -6,8 +6,7 @@
 # and with static archives the linker keeps whichever it meets first, silently.
 #
 # One query, empty output means clean; otherwise it prints one offending dependency path. Extra
-# arguments are passed to bazel (CI passes its --config flags). The linked binary's symbols are
-# checked separately by //src/workerd/server:rust-io-link-check.
+# arguments are passed to bazel (CI passes its --config flags).
 set -euo pipefail
 cd "$(dirname "$0")/.."
 

src/workerd/io/BUILD.bazel

@@ -102,6 +102,7 @@ wd_cc_library(
         ":observer",
         ":release-version",
         ":trace",
+        ":tracked-wasm-instance",
         ":wasm-instantiate-shim",
         ":worker-interface",
         ":worker-source",
@@ -356,15 +357,28 @@ wd_cc_library(
 
 wd_cc_library(
     name = "limit-enforcer",
-    srcs = ["tracked-wasm-instance.c++"],
     hdrs = [
         "limit-enforcer.h",
-        "tracked-wasm-instance.h",
     ],
     visibility = ["//visibility:public"],
     deps = [
         ":outcome_capnp",
-        "//src/workerd/jsg",
+        "@capnp-cpp//src/kj",
+        "@capnp-cpp//src/kj:kj-async-core",
+        "@workerd-v8//:v8",
+    ],
+)
+
+wd_cc_library(
+    name = "tracked-wasm-instance",
+    srcs = ["tracked-wasm-instance.c++"],
+    hdrs = [
+        "tracked-wasm-instance.h",
+    ],
+    visibility = ["//visibility:public"],
+    deps = [
+        "@capnp-cpp//src/kj",
+        "@workerd-v8//:v8",
     ],
 )
 
@@ -595,7 +609,7 @@ wd_test(
 kj_test(
     src = "tracked-wasm-instance-test.c++",
     deps = [
-        ":limit-enforcer",
+        ":tracked-wasm-instance",
     ],
 )
 

src/workerd/io/limit-enforcer.h

@@ -5,7 +5,6 @@
 #pragma once
 
 #include <workerd/io/outcome.capnp.h>
-#include <workerd/io/tracked-wasm-instance.h>
 
 #include <v8-isolate.h>
 
@@ -18,6 +17,7 @@
 namespace workerd {
 class IsolateObserver;
 class RequestObserver;
+class TrackedWasmInstanceList;
 
 struct ActorCacheSharedLruOptions;
 class IoContext;

src/workerd/io/worker.c++

@@ -16,6 +16,7 @@
 #include <workerd/io/frankenvalue.h>
 #include <workerd/io/per-isolate-bootstrap.h>
 #include <workerd/io/tracer.h>
+#include <workerd/io/tracked-wasm-instance.h>
 #include <workerd/io/validation.h>
 #include <workerd/io/wasm-instantiate-shim.embed.h>
 #include <workerd/io/worker-source-stats.h>

src/workerd/server/AGENTS.md

@@ -126,7 +126,7 @@ Bazel labels: `//src/workerd/server:workerd` (and `:workerd_cross`, which `wd_te
 - Never depend on the `@capnp-cpp//src/kj:kj-async` umbrella: it drags in `kj-async-os`, whose
   definitions collide with the shim's (an ODR violation). Use `:kj-async-core` / `:kj-async-io`.
   `just check-io-backend-graph` (one `bazel cquery somepath`, run by the lint CI lane) rejects any
-  such path from `:workerd`; `:rust-io-link-check` inspects the linked binary's symbols on unix.
+  such path from `:workerd`.
 - Ownership: the returned `kj::AsyncIoContext` borrows a heap holder attached to its
   `lowLevelProvider`; the holder owns the tokio context and the inert event port, so the context's
   references are valid for its whole lifetime and torn down once. The inert `kj::UnixEventPort` is

src/workerd/server/BUILD.bazel

@@ -1,7 +1,6 @@
 load("@bazel_skylib//lib:selects.bzl", "selects")
 load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
-load("@rules_shell//shell:sh_test.bzl", "sh_test")
 load("//:build/kj_test.bzl", "kj_test")
 load("//:build/linking.bzl", "CC_LIBRARY_LINKSTATIC")
 load("//:build/wd_capnp_library.bzl", "wd_capnp_library")
@@ -134,30 +133,6 @@ wd_cc_library(
     ],
 )
 
-# The link-time complement of build/rust_io_graph_check.sh: that query forbids dependency EDGES to
-# the kj OS I/O layer in the graph; this test inspects the symbols of the linked :workerd binary and
-# fails if kj's own setupAsyncIo()/OS provider made it in anyway (or the tokio shim did not). It
-# exists because a binary that silently links kj-async-os and lets kj's setupAsyncIo() win the
-# link passes every test that constructs its own event loop. Unix, unstripped only.
-sh_test(
-    name = "rust-io-link-check",
-    # One grep pass over the unstripped workerd binary: seconds locally, longer on CI runners.
-    size = "large",
-    srcs = ["//build/fixtures:rust_io_link_check.sh"],
-    # rlocationpath, not location: the binary is looked up under $TEST_SRCDIR so the check also
-    # works when workerd is built as an external repository (the internal build).
-    args = ["$(rlocationpath :workerd)"],
-    data = [":workerd"],
-    # Needs an unstripped binary: the ASAN lane strips (--strip=always), so it is excluded there
-    # by tag (test:asan filters -no-asan) rather than passing vacuously.
-    tags = ["no-asan"],
-    target_compatible_with = select({
-        # The script matches Itanium-mangled names; MSVC mangling is not covered.
-        "@platforms//os:windows": ["@platforms//:incompatible"],
-        "//conditions:default": [],
-    }),
-)
-
 wd_cc_library(
     name = "alarm-scheduler",
     srcs = [

src/workerd/server/factory/worker-factory.c++

@@ -14,6 +14,7 @@
 #include <workerd/io/limit-enforcer.h>
 #include <workerd/io/trace-stream.h>
 #include <workerd/io/tracer.h>
+#include <workerd/io/tracked-wasm-instance.h>
 #include <workerd/io/worker-entrypoint.h>
 #include <workerd/io/worker-fs.h>
 #include <workerd/jsg/snapshot.h>

src/workerd/tests/test-fixture.c++

@@ -13,6 +13,7 @@
 #include <workerd/io/limit-enforcer.h>
 #include <workerd/io/observer.h>
 #include <workerd/io/tracer.h>
+#include <workerd/io/tracked-wasm-instance.h>
 #include <workerd/io/worker-entrypoint.h>
 #include <workerd/jsg/jsg.h>
 #include <workerd/jsg/setup.h>

src/workerd/util/setup-async-io.c++

@@ -22,9 +22,8 @@
 // If kj-async-os is ever accidentally linked in, this TU's definitions collide with the native
 // ones, and with static archives the winner is LINK-ORDER-DEPENDENT -- a duplicate-symbol error if
 // you are lucky, the native setupAsyncIo silently winning (wrong event loop) or pairing with this
-// inert UnixEventPort (KJ_UNIMPLEMENTED at runtime) if you are not. The guards against that are
-// build/rust_io_graph_check.sh (the dependency graph) and //src/workerd/server:rust-io-link-check
-// (the linked binary's symbols).
+// inert UnixEventPort (KJ_UNIMPLEMENTED at runtime) if you are not. The guard against that is
+// build/rust_io_graph_check.sh (based on bazel aquery).
 
 #include <kj-rs-io/async-io.h>