Apply review feedback to AGENTS files

AGENTS.md

@@ -4,6 +4,14 @@
 
 # Verilator Guidelines for AI Coding Agents
 
+These files are the general layer an agent loads first -- nearest file wins, so
+you read this repository-root file plus the one for the directory you are
+editing. They stay deliberately high-level: where to start, how the tree is laid
+out, and the conventions reviewers otherwise enforce by hand. They are an index,
+not the architecture reference -- for depth (how a pass works internally, the
+algorithms, node lifetime) they point you to `docs/internals.rst`. When the
+guidance here is not enough, that is where to look next.
+
 This file has two parts. **Orientation** gets you productive in the codebase from
 a cold start. **Before you open a PR** is the checklist of conventions reviewers
 otherwise have to enforce by hand -- read it before submitting any change.
@@ -41,8 +49,7 @@ tree). In source order:
 | Emit | Lower the final AST to generated C++ | `V3EmitC*` |
 | Runtime | Library the generated model links against | `include/verilated*` |
 
-`docs/internals.rst` is the authoritative architecture and pass reference. Read
-it before any non-trivial change.
+This table is the map; `docs/internals.rst` has the detail behind each stage.
 
 ## Where to make a change
 
@@ -65,7 +72,8 @@ top-of-file comment.
 - Build in the source tree: `autoconf && ./configure && make -j8`. Configure with
   `--enable-ccwarn` so a new compiler warning stops the build.
 - Run one test from the repository root: `test_regress/t/t_<name>.py`.
-- Run the full regression: `make test`.
+- Run the full regression with `make test`. The complete suite requires
+  configuring with `--enable-longtests` (works on every OS, including macOS).
 
 ---
 
@@ -82,8 +90,12 @@ top-of-file comment.
       own reproducer when possible.
 - [ ] New code aims for 100% line coverage; branch coverage far below line coverage
       signals guards callers never violate -- justify or remove them.
-- [ ] Ran `make format` (clang-format) and `make cppcheck`; self-reviewed the diff
-      for leftover debug code, stale comments, and copy-paste errors.
+- [ ] Ran `make format` (clang-format), `make cppcheck`, and `make lint-py`;
+      self-reviewed the diff for leftover debug code, stale comments, and
+      copy-paste errors.
+- [ ] Ran the full regression on at least one OS before submitting. Partial runs
+      are fine during development, but the submitted PR is expected to pass every
+      test.
 - [ ] Did not edit `docs/CONTRIBUTORS` (humans only) or `Changes` (maintainer
       updates it near release).
 
@@ -109,8 +121,9 @@ The API you choose determines which test must accompany the change.
 
 ## Cross-cutting code rules
 
-- [ ] No non-ASCII characters in C++ sources or headers -- `--` not em dashes,
-      plain `'` not smart quotes. At write time, not when CI complains.
+- [ ] No non-ASCII characters in C++ sources or headers: write `--` (two ASCII
+      hyphens) rather than a Unicode em-dash, and a plain `'` rather than a smart
+      quote. At write time, not when CI complains.
 - [ ] Lists stay sorted: lexer/parser tokens, option declarations, enum values,
       configure feature lists, documented option lists.
 - [ ] `bin/` scripts are Python (distributed cross-platform); `nodist/` may use

include/AGENTS.md

@@ -34,7 +34,7 @@ ships to users, runs every simulation cycle, and must stay portable and fast.
 - **No exceptions in runtime code** -- use error returns or assertions; exceptions
   add overhead on every path.
 - **Use fixed-width model types** (`CData`/`SData`/`IData`/`QData`/`VlWide`), never
-  `size_t`, for model data. Process wide data word-by-word (`VL_MEMSET_W`,
+  `size_t`, for model data. Process wide data word-by-word (`VL_ZERO_W`,
   `VL_MEMCPY_W`), never bit-by-bit or byte-by-byte.
 - **Do all string parsing at verilation time** -- never parse strings during
   simulation; emit structured data or compile-time hints instead.

src/AGENTS.md

@@ -44,14 +44,14 @@ first. This file has two parts: **Orientation** explains the AST and pass model;
 
 ## Code style
 
-- Mark every variable, parameter, and pointer `const` where possible.
+- Mark every variable, parameter, pointer, and member function `const` where possible.
 - Pointer variables take a `p` suffix and pointer locals are doubly const where
   possible: `AstVar* const varp`; non-pointers never use the `p` suffix.
 - Do not use `auto` except for iterators or genuinely unwieldy types.
-- Use pre-increment (`++i`), never post-increment.
+- Use pre-increment (`++i`) unless you specifically need post-increment's old value.
 - Brace-initialize node and struct construction: `new AstIf{fl, condp, thenp, elsep}`.
-- No C-style casts -- `static_cast<T>` for non-AST types, `VN_AS`/`VN_CAST` for
-  AST downcasts.
+- Never use C-style casts; instead use `static_cast<T>` for non-AST types and
+  `VN_AS`/`VN_CAST` for AST downcasts.
 - `static constexpr` for compile-time constants, not `#define` or file-scope const.
 - Mark every `class`/`struct` `final` or `VL_NOT_FINAL` -- a distribution test
   scans all definitions.
@@ -62,7 +62,6 @@ first. This file has two parts: **Orientation** explains the AST and pass model;
 - Start every new `.cpp` with a top-of-file comment explaining the algorithm.
 - Comments are capitalized sentences written for an unknown future reader, without
   "I"/"we"/"our"; remove commented-out code -- version control preserves history.
-- Start continuation lines with the operator (`&&` at line start).
 - No `using namespace`; prefix non-namespaced symbols with `VL`/`Vl`.
 - Prefer semantic predicates over enum comparisons: `varp->isClassMember()`, not
   `varp->varType() == VVarType::MEMBER`.
@@ -99,8 +98,10 @@ first. This file has two parts: **Orientation** explains the AST and pass model;
 - Pointers to nodes outside op1p-op4p require a `broken()` override and
   `cloneRelink()` support -- avoid storing out-of-tree node pointers when possible.
 - Never allocate AstNode objects on the stack or by value -- always pointers.
-- Prefer `nodep->foreach(...)` and named accessors over `op1p()`..`op4p()`, but
-  verify traversal order is preserved when converting manual iteration.
+- Prefer a new `visit()` in an existing visitor over `nodep->foreach(...)` --
+  better for runtime, and handles diverse node types better. Prefer named
+  accessors over `op1p()`..`op4p()`, and verify traversal order is preserved
+  when converting manual iteration.
 - Prefer `AstForeach` over generating unrolled loop bodies -- constant-size code
   instead of O(N); wrap the body in `AstBegin` for scope isolation.
 - Always `skipRefp()` when comparing or resolving dtypes -- missing it breaks
@@ -177,7 +178,7 @@ first. This file has two parts: **Orientation** explains the AST and pass model;
   future parallelism.
 - Use Verilator's fixed-width data types for model data (`CData`/`SData`/`IData`/
   `QData`/`VlWide`), not `size_t`. Process wide data word-by-word
-  (`VL_MEMSET_W`, `VL_MEMCPY_W`), never bit-by-bit.
+  (`VL_ZERO_W`, `VL_MEMCPY_W`), never bit-by-bit.
 - No exceptions in verilated runtime code; do string parsing at verilation time,
   never during simulation.
 - Wrap unlikely hot-path branches in `VL_UNLIKELY`/`VL_LIKELY`.
@@ -215,9 +216,9 @@ first. This file has two parts: **Orientation** explains the AST and pass model;
 
 | File | Rule |
 |---|---|
-| `src/V3Options.cpp` | Chain `.notNeededForRerun()` onto `DECL_OPTION()` for options that do not affect semantic output |
+| `src/V3Options.cpp` | Chain `.notForRerun()` onto `DECL_OPTION()` for options that do not affect semantic output |
 | `src/V3Ast.cpp` | For composite types (queues, dynamic arrays) use `computeCastableImp()` on subtypes -- shallow `width()`/`similarDType()` checks miss nested incompatibility |
-| `src/V3AstNode*.h` | Every node class gets a what-construct comment and every member a semantic-purpose comment; put enum type definitions in `V3NodeAttr.h` |
+| `src/V3AstNode*.h` | Every node class gets a what-construct comment and every member a semantic-purpose comment; put enum type definitions in `V3AstAttr.h` |
 | `src/V3AstNodeExpr.h` | `CCast` is only for basic C types (char/short/int/QData) -- never 4-state logic or structs |
 | `src/V3AstNodeOther.h` | `cloneRelink` must propagate all stateful flags (e.g. `maybePointedTo`) and update internal references |
 | `src/V3Const.cpp` | Check `keepIfEmpty` before removing empty functions -- flagged functions must survive for codegen/side effects |