-
Notifications
You must be signed in to change notification settings - Fork 119
daslang: -log-compile-time CLI flag for per-module compile diagnostics #2680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| --- | ||
| slug: add-cli-flag-to-daslang-executable | ||
| title: How do I add a new command-line flag to the daslang executable that maps to a CodeOfPolicies field? | ||
| created: 2026-05-16 | ||
| last_verified: 2026-05-16 | ||
| links: [] | ||
| --- | ||
|
|
||
| Touch four spots in `utils/daScript/main.cpp`: | ||
|
|
||
| **1. File-scope static** (near the other flag statics around lines 34-57): | ||
|
|
||
| ```cpp | ||
| static bool logModuleCompileTime = false; | ||
| ``` | ||
|
|
||
| **2. Argv parser branch** in the main argv loop (`for ( int i=1; i < argc; ++i ) { if ( argv[i][0]=='-' ) { string cmd(argv[i]+1); ...`). Add an `else if` near related flags: | ||
|
|
||
| ```cpp | ||
| } else if ( cmd=="log-compile-time" ) { | ||
| logModuleCompileTime = true; | ||
| ``` | ||
|
|
||
| Single-dash style (`-log-compile-time`) for short or hyphenated flags. Double-dash (`--track-smart-ptr`) used for some longer ones. Existing convention is mostly single-dash — match the neighbors. | ||
|
|
||
| **3. Wire to policies in BOTH population sites:** | ||
|
|
||
| There are TWO CodeOfPolicies setup paths in `main.cpp`: | ||
| - `getPolicies()` near top of file — used by the AOT path (`das_aot_main`) | ||
| - `compile_and_run()` later — used by the normal run / JIT path | ||
|
|
||
| Both need the new line: | ||
|
|
||
| ```cpp | ||
| policies.log_module_compile_time = logModuleCompileTime; | ||
| ``` | ||
|
|
||
| If you only update one, the flag silently does nothing in the other mode. (Easy to verify with `git grep -n "policies\.no_lint" utils/daScript/main.cpp` — every existing flag is set at both sites.) | ||
|
|
||
| **4. Help text** in `print_help()`: | ||
|
|
||
| ```cpp | ||
| << " -log-compile-time log detailed per-module compile-time breakdown (parse/infer/opt/macro/simulate)\n" | ||
| ``` | ||
|
|
||
| **Optional: also wire daslang-live** if the flag matters for the live-reload host. Its argv parser is `utils/daslang-live/main.cpp:648-680` and its `CodeOfPolicies` is constructed inside `compile_script()` ~line 171. Skip if the flag is daslang-only. | ||
|
|
||
| **Rebuild target:** `cmake --build build --config Release --target daslang -j 64` (≈30s incremental). The static lives in `daslang.cpp`'s translation unit — no header changes, no library re-link of dependents. If you also touched `module_builtin_rtti.cpp` for the underlying policy registration, the full target list grows (rebuild test_aot if you care about AOT path). | ||
|
|
||
| ## Questions | ||
| - How do I add a new command-line flag to the daslang executable that maps to a CodeOfPolicies field? | ||
46 changes: 46 additions & 0 deletions
46
mouse-data/docs/add-field-to-codeofpolicies-full-checklist.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| --- | ||
| slug: add-field-to-codeofpolicies-full-checklist | ||
| title: What's the full checklist for adding a new field to CodeOfPolicies so it works end-to-end (compiler reads it, options.das_root maps it, RTTI sees it, das2rst documents it)? | ||
| created: 2026-05-16 | ||
| last_verified: 2026-05-16 | ||
|
borisbat marked this conversation as resolved.
|
||
| links: [] | ||
| --- | ||
|
|
||
| Adding a `CodeOfPolicies` field looks like one edit but is actually FOUR coordinated changes. Miss any of them and you get silent failures (no compile error, just wrong rendered docs / option not visible to RTTI). | ||
|
|
||
| **1. Declare the field** in `include/daScript/ast/ast.h` inside `struct CodeOfPolicies` (~line 1489). Annotate `/*option*/` if you want it overridable via per-file `options foo = true`: | ||
|
|
||
| ```cpp | ||
| /*option*/ bool log_module_compile_time = false; // short trailing comment is fine | ||
| ``` | ||
|
|
||
| **2. Register it for RTTI** in `src/builtin/module_builtin_rtti.cpp` (search for the existing `addField<DAS_BIND_MANAGED_FIELD(...)>("...")` calls in the CodeOfPolicies registration block, ~line 940): | ||
|
|
||
| ```cpp | ||
| addField<DAS_BIND_MANAGED_FIELD(log_module_compile_time)>("log_module_compile_time"); | ||
| ``` | ||
|
|
||
| Without this, `for_each_field` (used by `daslib/rst.das`) doesn't see the field and `das2rst` silently skips it. | ||
|
|
||
| **3. Add a description line** in `doc/source/stdlib/handmade/structure_annotation-rtti-CodeOfPolicies.rst`. This file is **strictly positional**: it maps line-by-line to RTTI-registered fields in *offset-sorted order* (which equals C++ declaration order for non-virtual structs). Place the new line at the position matching where you declared the field in step 1. | ||
|
|
||
| **Silent shift trap:** if the handmade file ends up short by ONE line vs registered fields, EVERY field after the gap gets its predecessor's description — but no error, no warning. The only way to spot it is reading the rendered `doc/source/stdlib/generated/rtti.rst` and confirming field name + description agree. | ||
|
|
||
| The reader code is `daslib/rst.das:817-880` (`document_topic`). It computes `headerLen = got - expected` and uses the first `headerLen` lines as preamble, then maps the remaining lines 1:1 to fields. So adding a line in the WRONG slot doesn't crash — it just silently mis-binds. | ||
|
|
||
| **4. Add a row** to the language-reference options table at `doc/source/reference/language/options.rst` (search for `log_total_compile_time` — it's a worked example near the bottom). Only needed if the field is `/*option*/` (user-overridable per file). | ||
|
|
||
| **Then regen + verify:** | ||
|
|
||
| ```bash | ||
| cmake --build build --config Release --target daslang -j 64 | ||
| bin/daslang doc/reflections/das2rst.das | ||
| grep -n "your_new_field" doc/source/stdlib/generated/rtti.rst # must show with correct description | ||
| ``` | ||
|
|
||
| If the description shown next to your field is for the WRONG field, your handmade file is short by one — find the missing description and add it. (I hit this in PR #2677: `max_call_depth` had been missing a description for who-knows-how-long, silently shifting ~30 later fields' descriptions up by one in the generated rtti.rst.) | ||
|
|
||
| Bonus: if you also want a CLI flag for your option in `daslang`, the wiring is described separately under `add-cli-flag-to-daslang-executable`. | ||
|
|
||
| ## Questions | ||
| - What's the full checklist for adding a new field to CodeOfPolicies so it works end-to-end (compiler reads it, options.das_root maps it, RTTI sees it, das2rst documents it)? | ||
37 changes: 37 additions & 0 deletions
37
mouse-data/docs/das2rst-handmade-file-positional-mapping-and-silent-shift-trap.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| slug: das2rst-handmade-file-positional-mapping-and-silent-shift-trap | ||
| title: How does daslib/rst.das map handmade .rst description lines to struct fields, and what is the silent-misalignment trap when one line is missing? | ||
| created: 2026-05-16 | ||
| last_verified: 2026-05-16 | ||
| links: [] | ||
| --- | ||
|
|
||
| `doc/source/stdlib/handmade/structure_annotation-<mod>-<Struct>.rst` files (and similar `class_annotation-*.rst`) are read **positionally** by `daslib/rst.das:817-880` (`document_topic`): | ||
|
|
||
| 1. Reader splits the file on `\n`, strips trailing blank lines → `got` lines. | ||
| 2. Compares to `expected = length(tab) - 1` (one row per field, after header row). | ||
| 3. Computes `headerLen = got - expected`. | ||
| 4. First `headerLen` lines printed verbatim as preamble (typically the struct description). | ||
| 5. **Last `expected` lines** mapped 1:1 to fields in **offset-sorted order** (= C++ declaration order for non-virtual structs). | ||
|
|
||
| So for a struct with N fields, you want N+1 non-blank lines (1 struct description + N field descriptions). The mapping is by POSITION, not by field name — there's no field-name annotation in the .rst, just one description per line. | ||
|
|
||
| **The silent shift trap:** if the file ends up with N lines instead of N+1 (one missing description anywhere in the middle), every field from the gap onwards inherits its successor's description. No compile error, no warning — `das2rst` happily produces garbage docs. The only way to detect: | ||
|
|
||
| ```bash | ||
| bin/daslang doc/reflections/das2rst.das | ||
| grep -A 1 "field_name_you_know" doc/source/stdlib/generated/rtti.rst # description text on next line must match | ||
| ``` | ||
|
|
||
| If the description belongs to a different field, your handmade file is short by one line somewhere between the file start and that field. Binary-search backwards to find where the shift begins — the first field with a wrong description marks the slot where one line is missing. | ||
|
|
||
| **Why this is fragile:** the file uses no field-name markers, no JSON, no positional separators. A future contributor adds a new C++ field, forgets the handmade entry → the next field's description (and everything after) drifts. Has happened multiple times to `CodeOfPolicies`. | ||
|
|
||
| **Fix recipe when you spot it:** read the offset of the misalignment in the rendered file, count back to find which C++ field has no description, add a line at the matching position in handmade. Repeat until rendered matches expected. | ||
|
|
||
| If `bin/daslang doc/reflections/das2rst.das` panics with "has less documentation than values" — that's the FRIENDLY case (file has FEWER lines than fields). The silent case is when somebody added the right number of lines but in the wrong positions, or when an EARLIER field's description is missing. | ||
|
|
||
| PR #2677 fixed a pre-existing silent shift in `structure_annotation-rtti-CodeOfPolicies.rst` (missing description for `max_call_depth`). About 30 fields downstream had their previous-field's description showing. | ||
|
|
||
| ## Questions | ||
| - How does daslib/rst.das map handmade .rst description lines to struct fields, and what is the silent-misalignment trap when one line is missing? |
35 changes: 35 additions & 0 deletions
35
...round-a-body-key-null-lookup-and-when-can-i-just-use-the-const-view-directly.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| --- | ||
| slug: when-do-i-need-unsafe-reinterpret-lt-jsonvalue-gt-around-a-body-key-null-lookup-and-when-can-i-just-use-the-const-view-directly | ||
| title: 'When do I need `unsafe(reinterpret<JsonValue?>(...))` around a `body?["key"] ?? null` lookup, and when can I just use the const view directly?' | ||
| created: 2026-05-16 | ||
|
borisbat marked this conversation as resolved.
|
||
| last_verified: 2026-05-16 | ||
| links: [] | ||
| --- | ||
|
|
||
| **Only when you store the pointer somewhere that wants `JsonValue?` (non-const-data).** For read-only operations (`is _string` / `as _string` / `write_json(v)` / `v != null` / `v.value is _object`) the const view from `?[]` works directly — no `unsafe` cast needed. | ||
|
|
||
| The `?[]` operator from `daslib/json_boost` has two overloads (json_boost.das:29-37): | ||
|
|
||
| ```das | ||
| def operator ?[] (a : JsonValue const? ==const; key : string) : JsonValue? const { ... } | ||
| def operator ?[] (var a : JsonValue? ==const; key : string) : JsonValue? { ... } | ||
| ``` | ||
|
|
||
| Indexing a `const`-bound view returns `JsonValue? const` (const-pointer wrapper, data is mutable). For reading the value out (the common case in any parser), this is fine. The `unsafe(reinterpret<JsonValue?>(...))` strips the outer const so you can: | ||
|
|
||
| - **Assign to a non-const struct field** — the load-bearing case. Example: `result.params = params_v` in `daslib/jsonrpc.das:188` requires `var params_v = unsafe(reinterpret<JsonValue?>(body?["params"] ?? null))`. Drop the reinterpret and you get `error[30915]: can only copy compatible type; JsonValue?& = JsonValue? const&`. | ||
| - **Pass to a function that takes `var` JsonValue?** — same root cause. | ||
|
|
||
| What does NOT need the cast (idiomatic): | ||
|
|
||
| ```das | ||
| let id_v = body?["id"] ?? null // const view | ||
| if (id_v != null && id_v.value is _string) // read OK | ||
| let s = id_v.value as _string // read OK | ||
| let j = write_json(id_v) // takes JsonValue? const, OK | ||
| ``` | ||
|
|
||
| Historical context: `live_api_stdio.das` (PR #2674, before the `daslib/jsonrpc` extraction in PR #2679) wrapped every `?[]` lookup in `unsafe(reinterpret<JsonValue?>(...))` defensively. When the same code moved into `daslib/jsonrpc.das`, four of the six call sites turned out to be read-only and the cast was dropped. Watch for this pattern next time you migrate JSON-traversal code into a library — the `unsafe` is usually noise. | ||
|
|
||
| ## Questions | ||
| - When do I need `unsafe(reinterpret<JsonValue?>(...))` around a `body?[\"key\"] ?? null` lookup, and when can I just use the const view directly? | ||
|
borisbat marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.