diff --git a/CLAUDE.md b/CLAUDE.md index d099e6c7ec..3653dee5ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -282,6 +282,18 @@ diagnostic in any tier. For path/filename ops use `fio` helpers (`base_name`/`dir_name`/`path_join`/...) - see `skills/daslang/references/files-and-paths.md`. Never hand-roll `rfind("/")` + slice: misses Windows separators. +**Return the result; do not fill an argument.** A function that hands its answer back +through a `var` parameter puts the caller's state in play at every call site, and a reader has +to open the callee to learn what moved. Return the value - a struct or a named tuple when +there is more than one. The carve-out is performance, and only measured performance: a +caller-owned buffer deliberately reused across calls on a hot path (inference kernels, audio +render callbacks, per-frame GPU/vertex buffers) stays an out-parameter, because returning a +fresh container there allocates per call. **LINT029 checks this and ships OFF** - arm it on a +file you are writing new code in with `options _lint = "LINT029"`, and treat what it says as +"be careful, keep this in mind" rather than a defect: reading a finding and moving on is a +normal outcome, not a suppression to justify. The dead-write half IS a gate - LINT023 is on +everywhere and reports a by-value out-parameter whose write nobody can read. + **Inline literals over temp-var-and-push:** for a short array consumed in one expression write `stack([a, b, c])`, not `var xs : array; xs |> emplace(a); xs |> emplace(b); stack(xs)`. Faster interpreted and easier to read; same for table literals and other bracketed constructors, while it stays readable. **Minimize `unsafe`:** most `unsafe(reinterpret)` in macro code exists to strip `const` from raw-pointer field access - fix the root cause by making the function parameter `var`, so field access returns non-const pointers. Reserve `unsafe` for genuinely unsafe operations (pointer arithmetic, `reinterpret` across unrelated types). diff --git a/daslib/ARCHITECTURE_LINT.md b/daslib/ARCHITECTURE_LINT.md index a126497025..c47768c03e 100644 --- a/daslib/ARCHITECTURE_LINT.md +++ b/daslib/ARCHITECTURE_LINT.md @@ -102,11 +102,35 @@ Companion to `ARCHITECTURE.md` in this folder; section numbers are unique across from dead code. Public symbols, lifecycle/macro hooks, virtual methods, and generic bodies are skipped: their callers are invisible to lint. - **Argument rules skip what the author does not own, and converge leaf-first.** - LINT012/013/014/023 stay silent on class methods, `[extern]` stubs, `finalize` + LINT012/013/014/023/029 stay silent on class methods, `[extern]` stubs, `finalize` overloads, and address-taken functions; LINT014 also skips a parameter whose mutability a callee demands - the leaf is flagged first and each fix re-exposes the next caller. A returned `var` argument keeps its `var` (a non-copyable result cannot move from const); a used `_name` parameter is never LINT004'd (interface and keyword-clash names). +- **LINT023 reads the store/use stream in order.** A use disqualifies a candidate only + once a store has been seen, so a read that precedes every store leaves the write dead - + the shape that hides a cleared-but-not-returned handle. `lint023_deferred_depth` covers + the placements whose execution order is not the source order: a closure, lambda or + generator body, and any loop body, where a read is taken to follow the store. A `label` + or a `goto` anywhere in the function sets `lint023_jumps` and the rule reports nothing + there at all: a backward jump can place a read after a store above it, so the order test + has no ground to stand on. It is + deliberately NOT `branch_depth`, which also counts `if` and `try/catch`: a conditional + reorders nothing, and the read that exposes the dead write sits inside one - folding the + two counters together makes the rule silent on the shape it exists to catch. +- **LINT029 ships default-off and is advisory.** `seed_default_disabled` carries it beside + STYLE005, so a tree sees it only once a file arms it (`options _lint = "LINT029"`) or the + repo config turns it on. The findings are a reminder to prefer a returned result, not a + defect report, so skipping one is a normal outcome rather than a suppression to justify. +- **LINT029 fires per argument straight from the compiler's access flags** + (`access_ref` / `access_info_pass_mutable` - the same evidence LINT014 reads in + reverse) and shares LINT014's return-erase. Receiver position exempts nothing: a + mutated struct is a finding wherever it sits, and only a struct whose every field + access yields a pointer or a handle stands down - there `var` is what keeps the + CONTAINED handle non-const. The rule stands down wholesale when the program source + sits in any `daslib/` folder (`lint029_source_exempt`, `daslib/lint_config.das`): + library code lives on the builder/state idiom (`var self`, `var writer`), so the + purity contract binds application code only. - **Closure bodies are per-rule, not global.** LINT010 counts a closure body as a branch (it may run later or never - writes inside must not kill outside stores, reads inside must not keep an outside init live); LINT021 counts the same body as an escape - a diff --git a/daslib/lint.das b/daslib/lint.das index 30074d1b4e..717a508715 100644 --- a/daslib/lint.das +++ b/daslib/lint.das @@ -158,6 +158,9 @@ class LintVisitor : AstVisitor { @do_not_delete lint014_candidates : array lint023_candidates : array lint023_store_nodes : table + @do_not_delete lint029_candidates : array + @do_not_delete lint029_mutated : array + lint029_exempt : bool = false //! AST visitor that performs lint checks. exprForTerminator : array compile_time_errors : bool @@ -177,6 +180,8 @@ class LintVisitor : AstVisitor { lint021_narrow_depth : int = 0 lint021_poison_depth : int = 0 lint021_closure_depth : int = 0 + lint023_deferred_depth : int = 0 + lint023_jumps : bool = false lint022_refs : RefIndex @do_not_delete lint022_self_struct : Structure const? = null lint022_generic_sweep : bool = false @@ -340,6 +345,8 @@ class LintVisitor : AstVisitor { lint021_narrow_depth = 0 lint021_poison_depth = 0 lint021_closure_depth = 0 + lint023_deferred_depth = 0 + lint023_jumps = false if (fun.moreFlags.isTemplate) { noLint = true return @@ -358,6 +365,10 @@ class LintVisitor : AstVisitor { if (idx >= 0) { lint014_candidates |> erase(idx) } + let idx29 = lint029_candidates |> find_index(ev.variable) + if (idx29 >= 0) { + lint029_candidates |> erase(idx29) + } } } @@ -370,13 +381,23 @@ class LintVisitor : AstVisitor { lint014_candidates |> clear() if (!noLint && genericDepth == 0) { for (c in lint023_candidates) { - if (!c.disqualified && c.stores > 0) { + if (!c.disqualified && c.stores > 0 && !lint023_jumps) { lint_error("LINT023: mutable by-value argument {c.v.name}: {describe(c.v._type, false, false, false)} is written but never read - the caller never sees a write to a by-value copy; declare it `{describe(c.v._type, false, false, false)}&` if it is an out-parameter, otherwise the write is dead", c.v.at) } } } lint023_candidates |> clear() lint023_store_nodes |> clear() + if (!noLint && genericDepth == 0) { + for (v in lint029_candidates) { + let structish = (v._type != null + && (v._type.baseType == Type.tStructure || v._type.baseType == Type.tHandle)) + continue if (structish && lint029_mutated |> find_index(v) < 0) + lint_error("LINT029: by-ref argument {v.name}: {describe(v._type, false, false, false)} is mutated - every call site sees the write; declare the parameter const and return the result instead if this function should be pure", v.at) + } + } + lint029_candidates |> clear() + lint029_mutated |> clear() if (!noLint && genericDepth == 0) { for (e in ds_entries) { if (!e.skip && e.has_pending && e.pending_pure && e.pending_expr != null) { @@ -405,6 +426,7 @@ class LintVisitor : AstVisitor { if (!noLint && genericDepth == 0 && (bf.isClosure || bf.isLambdaBlock || bf.isGeneratorBlock)) { branch_depth++ lint021_closure_depth++ + lint023_deferred_depth++ } } def override visitExprBlock(var blk : ExprBlock?) : ExpressionPtr { @@ -415,6 +437,7 @@ class LintVisitor : AstVisitor { if (!noLint && genericDepth == 0 && (bf.isClosure || bf.isLambdaBlock || bf.isGeneratorBlock)) { branch_depth-- lint021_closure_depth-- + lint023_deferred_depth-- } return <- blk } @@ -449,6 +472,9 @@ class LintVisitor : AstVisitor { exprForTerminator |> pop() } exprForTerminator |> push(0ul) + if (!noLint && genericDepth == 0) { + lint023_jumps = true + } } def override preVisitExprCast(expr : ExprCast?) : void { @@ -540,6 +566,19 @@ class LintVisitor : AstVisitor { lint014_candidates |> push(v) } + def lint029_collect_candidate(fun : FunctionPtr; v : VariablePtr) { + if (lint029_exempt || noLint || genericDepth > 0 || v.flags.generated || v.flags.marked_used || v.isAccessUnused) return + let name = string(v.name) + if ((name |> starts_with("_")) || find(name, "`") >= 0 + || fun.name == "finalize" || fun.moreFlags.addressTaken + || v._type == null || v._type.flags.constant + || !(v._type.flags.ref || v._type.isRefType) + || !(v.access_flags.access_ref || v.access_info.access_info_pass_mutable)) { + return + } + lint029_candidates |> push(v) + } + def lint023_collect_candidate(v : VariablePtr) { if (noLint || genericDepth > 0 || v.flags.generated || v.flags.marked_used || v.isAccessUnused) return let name = string(v.name) @@ -575,7 +614,7 @@ class LintVisitor : AstVisitor { if (ci < 0 || lint023_candidates[ci].disqualified) return if (expr.varFlags.under_clone || key_exists(lint023_store_nodes, intptr(expr))) { lint023_candidates[ci].stores++ - } else { + } elif (lint023_candidates[ci].stores > 0 || lint023_deferred_depth > 0) { lint023_candidates[ci].disqualified = true } } @@ -591,6 +630,25 @@ class LintVisitor : AstVisitor { validate_argument(arg, "LINT012", "function argument") validate_unwritten_var_argument(fun, arg) lint023_collect_candidate(arg) + lint029_collect_candidate(fun, arg) + } + + def private lint029_note_mutation(v : Variable const?) : void { + if (v != null && lint029_candidates |> find_index(v) >= 0 + && lint029_mutated |> find_index(v) < 0) { + lint029_mutated |> push(v) + } + } + + def override preVisitExprField(expr : ExprField?) : void { + if (noLint || genericDepth > 0 || expr.value == null) return + var obj = expr.value + if (obj is ExprRef2Value) { + obj = (obj as ExprRef2Value).subexpr + } + return if (!(obj is ExprVar) || (expr._type != null + && (expr._type.baseType == Type.tPointer || expr._type.baseType == Type.tHandle))) + lint029_note_mutation((obj as ExprVar).variable) } def override preVisitExprBlockArgument(_blk : ExprBlock?; arg : VariablePtr; _lastArg : bool) : void { @@ -1064,6 +1122,9 @@ class LintVisitor : AstVisitor { lint022_refs.names |> insert("{expr.name}") } if (noLint || genericDepth > 0) return + if (key_exists(lint023_store_nodes, intptr(expr))) { + lint029_note_mutation(expr.variable) + } lint023_note_use(expr) let ci = lint021_find(expr.variable) if (ci >= 0 && !lint021_candidates[ci].disqualified) { @@ -1122,18 +1183,38 @@ class LintVisitor : AstVisitor { return <- expr } + //! A label or a goto puts control flow beyond what the source order describes - a backward + //! jump can place a read after a store that appears above it - so the order test cannot judge + //! this function and LINT023 stands down for all of it. + def override preVisitExprGoto(_expr : ExprGoto?) : void { + if (!noLint && genericDepth == 0) { + lint023_jumps = true + } + } def override preVisitExprFor(_expr : ExprFor?) : void { - if (!noLint && genericDepth == 0) branch_depth++ + if (!noLint && genericDepth == 0) { + branch_depth++ + lint023_deferred_depth++ + } } def override visitExprFor(var expr : ExprFor?) : ExpressionPtr { - if (!noLint && genericDepth == 0) branch_depth-- + if (!noLint && genericDepth == 0) { + branch_depth-- + lint023_deferred_depth-- + } return <- expr } def override preVisitExprWhile(_expr : ExprWhile?) : void { - if (!noLint && genericDepth == 0) branch_depth++ + if (!noLint && genericDepth == 0) { + branch_depth++ + lint023_deferred_depth++ + } } def override visitExprWhile(var expr : ExprWhile?) : ExpressionPtr { - if (!noLint && genericDepth == 0) branch_depth-- + if (!noLint && genericDepth == 0) { + branch_depth-- + lint023_deferred_depth-- + } return <- expr } def override preVisitExprTryCatch(_expr : ExprTryCatch?) : void { @@ -1169,6 +1250,7 @@ def public paranoid(prog : ProgramPtr; compile_time_errors : bool; disabled_code //! ``// nolint:CODE`` directives. var astVisitor = new LintVisitor( compile_time_errors = compile_time_errors, + lint029_exempt = lint029_source_exempt(prog), multiple_contexts = prog._options |> find_arg("multiple_contexts") ?as tBool ?? prog.policies.multiple_contexts) astVisitor.disabled_codes := disabled_codes make_visitor(*astVisitor) $(adapter) { @@ -1194,6 +1276,7 @@ def public paranoid_collect(prog : ProgramPtr; var errors : array; var astVisitor = new LintVisitor( compile_time_errors = false, collect_errors = true, + lint029_exempt = lint029_source_exempt(prog), multiple_contexts = prog._options |> find_arg("multiple_contexts") ?as tBool ?? prog.policies.multiple_contexts) astVisitor.disabled_codes := disabled_codes astVisitor.enabled_codes := enabled_codes @@ -1216,6 +1299,7 @@ def public paranoid_collect_issues(prog : ProgramPtr; var issues : array find_arg("multiple_contexts") ?as tBool ?? prog.policies.multiple_contexts) astVisitor.disabled_codes := disabled_codes astVisitor.enabled_codes := enabled_codes diff --git a/daslib/lint_config.das b/daslib/lint_config.das index abd51d032d..d16dac0861 100644 --- a/daslib/lint_config.das +++ b/daslib/lint_config.das @@ -108,10 +108,12 @@ def public load_lint_config_from_path(path : string; var disabled_codes : table< } } -//! Seeds ``disabled_codes`` with the canonical default-off rule set (currently just STYLE005). Call before ``load_lint_config`` so that a ``STYLE005 = true`` directive in ``.lint_config`` can re-enable it. +//! Seeds ``disabled_codes`` with the canonical default-off rule set (STYLE005 and LINT029). Call before ``load_lint_config`` so that a ``STYLE005 = true`` directive in ``.lint_config`` can re-enable it. def public seed_default_disabled(var disabled_codes : table) : void { - if (!key_exists(disabled_codes, "STYLE005")) { - disabled_codes |> insert("STYLE005") + for (code in ["STYLE005", "LINT029"]) { + if (!key_exists(disabled_codes, code)) { + disabled_codes |> insert(code) + } } } @@ -218,9 +220,35 @@ def public build_lint_macro_disabled(prog : ProgramPtr) : table { if (key_exists(disabled, code)) disabled |> erase(code) } load_env_disabled(disabled) + add_module_rule_overrides(prog, disabled) return <- disabled } +//! The module-local rule overrides, layered LAST so the file has the final say: ``options _nolint`` +//! silences the listed codes for this file, ``options _lint`` arms them (a default-off rule, or one +//! the repo config turned off). An enable beats a disable; an unknown code changes nothing. +def public add_module_rule_overrides(prog : ProgramPtr; var disabled_codes : table) : void { + apply_module_codes(prog, "_nolint", disabled_codes, true) + apply_module_codes(prog, "_lint", disabled_codes, false) +} + +def private apply_module_codes(prog : ProgramPtr; option : string; + var disabled_codes : table; disable : bool) : void { + let configured = prog._options |> find_arg(option) + return if (!(configured is tString)) + for (part in split(configured as tString, ",")) { + let code = part |> strip |> to_upper + continue if (empty(code)) + if (disable) { + if (!key_exists(disabled_codes, code)) { + disabled_codes |> insert(code) + } + } elif (key_exists(disabled_codes, code)) { + disabled_codes |> erase(code) + } + } +} + def private module_source_path(prog : ProgramPtr) : string { let mod = prog.getThisModule if (mod == null) return "" @@ -243,6 +271,10 @@ def public is_daslib_source(prog : ProgramPtr) : bool { //! True when the compiling module's source lives under ``daslib/`` or ``modules/`` - //! shipped library code whose emitted strings flow into every consumer's logs (STYLE039). +def public lint029_source_exempt(prog : ProgramPtr) : bool { + return path_has_segment(module_source_path(prog), "daslib") +} + def public is_shipped_library_source(prog : ProgramPtr) : bool { let file = module_source_path(prog) return path_has_segment(file, "daslib") || path_has_segment(file, "modules") diff --git a/doc/reflections/das2rst.das b/doc/reflections/das2rst.das index 3aafcfcadf..397ee80490 100644 --- a/doc/reflections/das2rst.das +++ b/doc/reflections/das2rst.das @@ -1364,9 +1364,9 @@ def document_module_lint(_root : string) { def document_module_lint_config(_root : string) { var mod = find_module("lint_config") var groups <- array( - group_by_regex("Configuration", mod, %regex~(load_lint_config|load_lint_config_from_path|load_env_disabled|seed_default_disabled|build_lint_macro_disabled|lint_config_forces_on|rule_docs_only_at)$%%), + group_by_regex("Configuration", mod, %regex~(load_lint_config|load_lint_config_from_path|load_env_disabled|seed_default_disabled|build_lint_macro_disabled|add_module_rule_overrides|lint_config_forces_on|rule_docs_only_at)$%%), group_by_regex("Path excludes", mod, %regex~(load_path_excludes_from_path|matches_path_excludes|is_lint_path_excluded)$%%), - group_by_regex("Path-based rule defaults", mod, %regex~(is_daslib_source|is_shipped_library_source|is_core_library_source)$%%), + group_by_regex("Path-based rule defaults", mod, %regex~(is_daslib_source|is_shipped_library_source|is_core_library_source|lint029_source_exempt)$%%), group_by_regex("Lint-surface predicates", mod, %regex~(is_user_authored_body|is_lint_fixture_name|lint_file_skip_reason)$%%), group_by_regex("Structured findings", mod, %regex~(make_lint_issue)$%%), group_by_regex("Format policy", mod, %regex~(format_policy_for|partition_by_format_policy)$%%) diff --git a/doc/source/reference/language/lint.rst b/doc/source/reference/language/lint.rst index 5ab4963dbe..a996d3a900 100644 --- a/doc/source/reference/language/lint.rst +++ b/doc/source/reference/language/lint.rst @@ -141,6 +141,19 @@ Add a ``// nolint:CODE`` comment on the same line as the flagged expression:: The suppression is exact: ``// nolint:PERF003`` only suppresses PERF003, not other rules. An optional explanation after the code is recommended but not required. +When one rule needs silencing throughout a file — a whole-file shape the rule +cannot see — declare it once in the header instead of on every line. The mirror +arms a rule for one file: a rule that is off by default, or one the repo +configuration turned off:: + + options _nolint = "LINT029,PERF030" + options _lint = "STYLE005" + +The codes are comma-separated and apply to that file only. Both are layered after +the repo configuration and the environment, so the file has the final say, and an +enable beats a disable of the same code. An unknown code is ignored, so a typo +changes nothing. + A file whose *subject* conflicts with the lint pipeline's compile policies can opt out entirely with a ``// lint-skip-file: `` comment in the file header (the first 16 lines — deeper occurrences are treated as prose, so quoting the directive @@ -1083,7 +1096,19 @@ rule: a read, ``return``, passing it on (to a ``&`` slot too — the callee may read it first), ``addr``, a lambda capture, a write through a pointer parameter's pointee (``p.x = 1`` reaches the caller's object; reassigning ``p`` itself is a store like any other). Class methods and ``[extern]`` stubs, -underscore-prefixed names and ``[unused_argument]`` are skipped. +underscore-prefixed names and ``[unused_argument]`` are skipped. A parameter +the body writes *and* reads back is silent: the writes cannot leave the +function, so the parameter simply is the local. + +A read only counts when it can **follow** a store. A read that precedes every +store — the ``if (tex != 0u)`` guard above a ``tex = 0u`` that clears a handle +the caller still holds — leaves the write dead, and the rule says so. Two +placements are read as "this runs after the store" whatever their position in +the source, because they can: a read inside a closure, lambda or generator body +(it runs when the block is invoked), and a read anywhere in a loop body (the +next iteration puts it after the store). A function carrying a ``label`` or a +``goto`` is skipped whole - a backward jump can put a read after a store that +sits above it, so source order describes nothing there. .. das-doc: alt .. code-block:: das @@ -1106,10 +1131,47 @@ underscore-prefixed names and ``[unused_argument]`` are skipped. return length(text) } - // Not flagged — a scratch parameter is written and then read - def twice(var n : int) : int { - n = n * 2 - return n +LINT029 — by-ref argument is mutated +==================================== + +The purity contract: a function that writes to a ``var`` by-ref parameter — a +struct, array or table passed ``var``, or an explicit ``&`` — changes state +its caller owns. Functions communicate through return values; a call site +where every argument is read-only is one a reader can reason about locally. +The rule ships **off**. It is advice, not a gate — a "be careful, keep this in +mind" reminder rather than a defect report. Arm it on a file you are writing new +code in with ``options _lint = "LINT029"``, read what it says, and skip the +findings you disagree with: a deliberate out-parameter keeps its ``var`` under a +per-line ``// nolint:LINT029``. A tree that wants the contract enforced for +everyone turns it on with ``LINT029 = true`` in ``.lint_config``. + +A parameter counts as mutated when the body writes it directly or passes it on +to a mutable slot. Receiver position exempts nothing: a mutated struct is a +finding wherever it sits. Four shapes are outside the rule. A parameter the +function returns — ``var`` is how a non-copyable value moves out (the same +exemption LINT014 grants). A struct whose every field access yields a pointer +or a handle — there ``var`` is what keeps the CONTAINED handle non-const, so +the const-parameter remedy does not exist. Block and lambda parameters — a +callback's mutable slot is its caller's contract. Library +code — any ``daslib/`` folder, the stdlib and module daslibs alike — is +exempt wholesale, because the builder/state idiom (``var self``, +``var writer``, ``var st``) is the library's own. Class methods, ``[extern]`` +stubs, ``finalize`` overloads, address-taken functions, underscore-prefixed +names and ``[unused_argument]`` are skipped. + +.. das-doc: alt +.. code-block:: das + + // Flagged when enabled — the caller's array grows + def append_one(var xs : array) { // LINT029 on xs + xs |> push(1) + } + + // Pure — the result is returned + def appended_one(xs : array) : array { + var r := xs + r |> push(1) + return <- r } .. _perf_lint: diff --git a/examples/games/river_run/rr_postfx.das b/examples/games/river_run/rr_postfx.das index cc2a41eac0..1584290024 100644 --- a/examples/games/river_run/rr_postfx.das +++ b/examples/games/river_run/rr_postfx.das @@ -360,14 +360,14 @@ def private make_texture(w, h : int; internal_format : uint; format, data_type, return tex } -def private drop_texture(var tex : uint) { +def private drop_texture(var tex : uint&) { if (tex != 0u) { glDeleteTextures(1, unsafe(addr(tex))) tex = 0u } } -def private drop_fbo(var fbo : uint) { +def private drop_fbo(var fbo : uint&) { if (fbo != 0u) { glDeleteFramebuffers(1, unsafe(addr(fbo))) fbo = 0u diff --git a/modules/dasLLAMA/REVIEW.das b/modules/dasLLAMA/REVIEW.das index d15f1a9956..8b5cb86b6d 100644 --- a/modules/dasLLAMA/REVIEW.das +++ b/modules/dasLLAMA/REVIEW.das @@ -461,6 +461,56 @@ def private check_exe_fn_global_restore { } } +// The ONE release, mechanically. A carrier's image backing is either a mapping or a chunk, and +// image_chunk_alloc hands back an image-page-aligned offset INTO a das-heap array - so +// munmap on a chunk SUCCEEDS and punches a hole in the process heap, which kills an unrelated +// malloc long afterwards with nothing to point at the caller. Only image_backing_release tells +// the two apart. fmap_close is therefore legal in exactly two places: inside that release, and +// inside a function that opened the mapping it closes. +def private report_stray_closes(path, fn : string; opened : bool; closes : array) { + return if (fn == "image_backing_release" || opened) + for (ln in closes) { + gate_finding(path, ln, + "fmap_close on a backing this function did not open - a carrier's image_map may be a chunk, and munmap on one succeeds and holes the heap; release it through image_backing_release") + } +} + +def private check_image_backing_release { + var files : array + for (folder in ["modules/dasLLAMA/dasllama", "modules/dasLLAMA/tests", "modules/dasLLAMA/benchmarks", + "modules/dasLLAMA/performance", "modules/dasLLAMA/harness", "utils/dasllama-server", + "utils/dasllama-convert", "tutorials/dasLLAMA"]) { + collect_das_files(folder, files) + } + for (p in files) { + var fn = "" + var opened = false + var closes : array + var line_no = 0 + for (line in split(strip_line_comments(fread(p)), "\n")) { + line_no++ + if (line |> starts_with("def ")) { + report_stray_closes(p, fn, opened, closes) + var rest = slice(line, 4) + if (rest |> starts_with("private ")) { + rest = slice(rest, length("private ")) + } + fn = ident_prefix(rest) + opened = false + closes |> clear() + } + if (contains_word(line, "fmap_open")) { + opened = true + } + if (contains_word(line, "fmap_close")) { + closes |> push(line_no) + } + } + report_stray_closes(p, fn, opened, closes) + } + delete files +} + [export] def main() : int { if (!fexist("modules/dasLLAMA/REVIEW.das") || !fexist(FACADE)) { @@ -471,6 +521,7 @@ def main() : int { check_race_bind_numbers() check_gpu_role_partition() check_fastmath_default() + check_image_backing_release() var inscope tut_texts : array read_das_stripped(TUTORIAL_DIR, tut_texts) var inscope rst_texts : array diff --git a/modules/dasLLAMA/tests/test_model_image.das b/modules/dasLLAMA/tests/test_model_image.das index 11c1673a88..d38ee06714 100644 --- a/modules/dasLLAMA/tests/test_model_image.das +++ b/modules/dasLLAMA/tests/test_model_image.das @@ -147,9 +147,7 @@ def finalize(var t : ImgProbe) { } } if (t.image_map != null) { - unsafe { - fmap_close(t.image_map, t.image_bytes) - } + image_backing_release(t.image_map, t.image_bytes) t.image_map = null t.image_bytes = 0ul } diff --git a/modules/dasTerminal/daslib/terminal.das b/modules/dasTerminal/daslib/terminal.das index 4aaa0e1fa9..a10b077735 100644 --- a/modules/dasTerminal/daslib/terminal.das +++ b/modules/dasTerminal/daslib/terminal.das @@ -1469,7 +1469,7 @@ def private checkpoint_cell_visible(cell : TerminalCell const) : bool { def private checkpoint_write_row(var writer : StringBuilderWriter; row : TerminalRow const; var active_style : TerminalCell; - var active_hyperlink : string) { + var active_hyperlink : string&) { var last_column = -1 for (column in range(length(row.cells))) { if (checkpoint_cell_visible(row.cells[column])) { @@ -1495,7 +1495,7 @@ def private checkpoint_write_buffer(var writer : StringBuilderWriter; buffer : TerminalBuffer const; include_history : bool; var active_style : TerminalCell; - var active_hyperlink : string) { + var active_hyperlink : string&) { var line = 0 let line_count = (include_history ? length(buffer.history) : 0) + length(buffer.screen) if (include_history) { @@ -1532,7 +1532,7 @@ def private checkpoint_write_cursor_state(var writer : StringBuilderWriter; def private checkpoint_restore_buffer_state(var writer : StringBuilderWriter; buffer : TerminalBuffer const; var active_style : TerminalCell; - var active_hyperlink : string) { + var active_hyperlink : string&) { let esc = to_char(27) writer |> write("{esc}[{buffer.scroll_top + 1};{buffer.scroll_bottom + 1}r") writer |> write("{esc}[{buffer.saved_cursor.row + 1};{buffer.saved_cursor.column + 1}H") diff --git a/utils/lint/main.das b/utils/lint/main.das index ee3e58f8fa..d15f8659a0 100644 --- a/utils/lint/main.das +++ b/utils/lint/main.das @@ -436,6 +436,15 @@ def set_skip_reason(file : string; lint_fixtures : bool; var result : LintResult return false } +def private file_nolint_codes(prog : ProgramPtr; base : table) : table { + var merged : table + for (code in keys(base)) { + merged |> insert(code) + } + add_module_rule_overrides(prog, merged) + return <- merged +} + def lint_file(file : string; run_paranoid, run_perf, run_style, comment_hygiene, lint_fixtures, lint_excluded_paths : bool; disabled_codes, enabled_codes : table; defer_stale : bool) : LintResult { var result = LintResult(file = file) @@ -482,19 +491,20 @@ def lint_file(file : string; run_paranoid, run_perf, run_style, comment_hygiene, result.compile_errors = issue_str return } + let file_disabled <- file_nolint_codes(program, disabled_codes) if (run_paranoid) { var paranoid_issues : array - result.count += paranoid_collect_issues(program, paranoid_issues, disabled_codes, enabled_codes) + result.count += paranoid_collect_issues(program, paranoid_issues, file_disabled, enabled_codes) result.issues |> push_from(paranoid_issues) } if (run_perf) { var perf_issues : array - result.count += perf_lint_collect_issues(program, perf_issues, disabled_codes, enabled_codes) + result.count += perf_lint_collect_issues(program, perf_issues, file_disabled, enabled_codes) result.issues |> push_from(perf_issues) } if (run_style) { var style_issues : array - result.count += style_lint_collect_issues(program, style_issues, disabled_codes, enabled_codes, comment_hygiene) + result.count += style_lint_collect_issues(program, style_issues, file_disabled, enabled_codes, comment_hygiene) result.issues |> push_from(style_issues) } } diff --git a/utils/lint/tests/lint023_unread_var_argument.das b/utils/lint/tests/lint023_unread_var_argument.das index 48e5c8db48..5203b2d512 100644 --- a/utils/lint/tests/lint023_unread_var_argument.das +++ b/utils/lint/tests/lint023_unread_var_argument.das @@ -13,7 +13,7 @@ options auto_inline_functions = false // lint fixtures assert SOURCE shapes; s // appearance (a read, `return`, passing on, addr-of, capture, a write through // a pointer's pointee) is a use. -expect 50503:9 +expect 50503:10 require daslib/lint @@ -71,8 +71,40 @@ def bad_ptr_reassign(var f : Foo?) { // LINT023 - the pointer itself is a by-v f = null } +def bad_read_before_store(var tex : uint) { // LINT023 - the read precedes the store, so nothing reads it back + if (tex != 0u) { + print("{tex}\n") + tex = 0u + } +} + // === Negatives === +def good_goto_backwards(var n : int; times : int) { + var k = 0 + label 1: + print("{n}\n") + n = k + k++ + if (k < times) { + goto label 1 + } +} + +def good_read_in_block(var n : int) { + n = 1 + invoke($() { + print("{n}\n") + }) +} + +def good_read_in_loop(var n : int) { + for (i in range(2)) { + print("{n}\n") + n = i + } +} + def good_write_then_read(var n : int) : int { n = n * 2 return n @@ -162,4 +194,12 @@ def main() { good_struct(s) good_underscore(1) good_marked(1) + drop_handle_probe() +} + +def private drop_handle_probe() { + bad_read_before_store(7u) + good_read_in_block(1) + good_read_in_loop(1) + good_goto_backwards(1, 3) } diff --git a/utils/lint/tests/lint029_by_ref_arg_mutation.das b/utils/lint/tests/lint029_by_ref_arg_mutation.das new file mode 100644 index 0000000000..6205bf221b --- /dev/null +++ b/utils/lint/tests/lint029_by_ref_arg_mutation.das @@ -0,0 +1,108 @@ +options gen2 +options auto_inline_functions = false +options _lint = "LINT029" // the rule ships default-off; the fixture arms it + +expect 50503:5 + +require daslib/lint + +struct private Box { + x : int +} + + +def bad_push(var xs : array) { + xs |> push(1) +} + +def bad_second_state(tag : string; var b : Box) { + b.x = length(tag) +} + +def bad_receiver(var b : Box; delta : int) { + b.x += delta +} + +def bad_ref_scalar(var n : int&) { + n = 5 +} + +def bad_pass_mutable(var xs : array) { + bad_push(xs) +} + + +def good_read_only(xs : array) : int { + return length(xs) +} + +def good_returned(var xs : array) : array { + return <- xs +} + +def good_pure_grow(xs : array) : array { + var r := xs + r |> push(1) + return <- r +} + +def good_nolint(var xs : array) { // nolint:LINT029 + xs |> push(2) +} + +struct private PtrPair { + a : int? + b : int? +} + +def good_ptr_fields(var pp : PtrPair) { + unsafe { + *pp.a = 1 + *pp.b = 2 + } +} + +def good_lambda_param() : int { + let l <- @(var v : array) { + v |> push(1) + } + var xs : array + l |> invoke(xs) + return length(xs) +} + +def apply_each(var xs : array; blk : block<(var v : int&) : void>) { // nolint:LINT029 + for (x in xs) { + blk |> invoke(x) + } +} + +def good_block_param() : int { + var xs <- [1, 2, 3] + xs |> apply_each() $(var v : int&) { + v += 1 + } + return xs[0] +} + +[export] +def main() { + var xs : array + bad_push(xs) + var b = Box() + bad_second_state("t", b) + bad_receiver(b, 1) + var n = 1 + bad_ref_scalar(n) + bad_pass_mutable(xs) + print("{good_read_only(xs)}\n") + var moved <- good_returned(xs) + var grown <- good_pure_grow(moved) + good_nolint(grown) + var pa = 0 + var pb = 0 + var pp = PtrPair(a = unsafe(addr(pa)), b = unsafe(addr(pb))) + good_ptr_fields(pp) + print("{pa + pb}\n") + print("{length(grown)} {good_lambda_param()} {good_block_param()}\n") +} diff --git a/utils/lint/tests/lint_module_rule_overrides.das b/utils/lint/tests/lint_module_rule_overrides.das new file mode 100644 index 0000000000..ee126ab8be --- /dev/null +++ b/utils/lint/tests/lint_module_rule_overrides.das @@ -0,0 +1,26 @@ +options gen2 +options auto_inline_functions = false +options _lint = "STYLE005" +options _nolint = "STYLE005" +// options _lint arms a rule the defaults turned off; options _nolint silences one. +// STYLE005 is default-off, so it fires here only because _lint names it - and it +// fires despite _nolint naming it too, which is the precedence: enable beats disable. + +expect 31209:2 + +require daslib/style_lint + +def private clamped(n : int) : int { + if (n < 0) { + return 0 + } + if (n > 100) { + return 100 + } + return n +} + +[export] +def main() { + print("{clamped(-1)} {clamped(500)} {clamped(7)}\n") +} diff --git a/web/examples/ui/samples/examples/river_run/rr_postfx.das b/web/examples/ui/samples/examples/river_run/rr_postfx.das index cc2a41eac0..1584290024 100644 --- a/web/examples/ui/samples/examples/river_run/rr_postfx.das +++ b/web/examples/ui/samples/examples/river_run/rr_postfx.das @@ -360,14 +360,14 @@ def private make_texture(w, h : int; internal_format : uint; format, data_type, return tex } -def private drop_texture(var tex : uint) { +def private drop_texture(var tex : uint&) { if (tex != 0u) { glDeleteTextures(1, unsafe(addr(tex))) tex = 0u } } -def private drop_fbo(var fbo : uint) { +def private drop_fbo(var fbo : uint&) { if (fbo != 0u) { glDeleteFramebuffers(1, unsafe(addr(fbo))) fbo = 0u