perf_lint: add PERF020 — redundant same-name workhorse cast#2753
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new performance lint rule (PERF020) to catch redundant same-type “workhorse” casts (e.g., int64(x) where x : int64) that the typer does not elide and which therefore remain as real ExprCall nodes.
Changes:
- Implement PERF020 detection in
daslib/perf_lintand ensure it runs even inside closures. - Add a dedicated dastest fixture for PERF020 and adjust the PERF019 fixture to avoid the new rule while still exercising PERF019’s unwrap path.
- Document PERF020 in both the skills guide and the Sphinx reference docs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| utils/lint/tests/perf020_redundant_cast.das | New dastest fixture covering redundant same-type casts across the 15 workhorse cast names. |
| utils/lint/tests/perf019_int_cast_collapse.das | Updates a “good” case to avoid triggering PERF020 while still testing PERF019’s nested-cast unwrap behavior. |
| skills/perf_lint.md | Adds PERF020 to the perf lint rules reference table with matching/exclusion notes. |
| doc/source/reference/language/lint.rst | Adds a new PERF020 section to the lint reference documentation. |
| daslib/perf_lint.das | Implements PERF020 (type mapping + call-site check) and wires it into preVisitExprCall before the closure guard; also removes a redundant uint64(intptr(...)) cast. |
| CLAUDE.md | Updates the “Don’t write / Write instead” migration table to include PERF020 guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e6822d4 to
29b8ee3
Compare
`T(x)` where `x` is already of workhorse type `T` lowers to an `ExprCall` against the cast-named overload (registered by `ADD_NUMERIC_CASTS` / `ADD_BITFIELD_CASTS` plus `das_lexical_cast_*`) — the typer does not elide it, so the no-op survives all the way to codegen. The rule fires for the 15 workhorse names: `int`/`int8`/`int16`/`int64`, `uint`/`uint8`/`uint16`/`uint64`, `float`, `double`, `string`, `bitfield`/`bitfield8`/`bitfield16`/`bitfield64`. Match: `call.func.fromGeneric?.name ?? call.func.name` is in the workhorse-cast set AND `arg._type.baseType` equals the target. Const/ ref/temp qualifiers on the argument are ignored — only `baseType` matters. User-named bitfield/enum ctors (`MyBitfield(x)`), vector ctors (`int2(x,y)`), and `string(das_string)` are out of scope by construction. Dispatch hoisted above the closure guard in `preVisitExprCall` so the rule fires anywhere — a redundant cast inside a closure body is still redundant. `perf_warning` already handles macro-generated / generic-instantiation suppression. Drive-by: drop the redundant `uint64(intptr(...))` cast at line 394 (intptr returns uint64 already) — self-hit found by the new rule. Fixture `utils/lint/tests/perf020_redundant_cast.das` covers 11 bad arms (one per category) and 9 good arms (widening, narrowing, signedness change, float<->int, user-named bitfield, vector ctor, das_string materialization). Uses parameters not constants so the runtime forms survive constant folding under dastest (same lesson as PERF019). Restructured `good_nested_cast` in the PERF019 fixture: the previous `int(int(m1))` shape was deliberately exercising PERF019's nested-call unwrap, but is itself a textbook PERF020 case. Replaced the inner `int(m1)` with an int64 widen so the same PERF019 code path is exercised without tripping PERF020. CI lint gate (`extended_checks.yml:225-238`) already lints every changed `.das` file — no workflow edits needed; the new rule rides the existing pipeline. A daslib sweep found ~30 PERF020 hits in 14 other files; those go to a follow-up cleanup PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
29b8ee3 to
6bf6817
Compare
aleksisch
reviewed
May 20, 2026
|
|
||
| def check_perf020_redundant_cast(call : ExprCall?) : void { | ||
| if (call == null || call.func == null || length(call.arguments) != 1) return | ||
| let fname = call.func.fromGeneric != null ? string(call.func.fromGeneric.name) : string(call.func.name) |
Collaborator
There was a problem hiding this comment.
string(a ? b : c) instead of a ? string(b) : string(c)? Can be added to linter too btw. It's not first time such code was written.
This was referenced May 20, 2026
borisbat
added a commit
that referenced
this pull request
May 20, 2026
…ssion PR #2753's new PERF020 rule (`T(x)` where x is already T) caught three real sloppy-codegen sources that all emitted unnecessary workhorse casts the interpreter doesn't fold: 1. **daslib/linq.das average()** (iter + array overloads, lines 1529 / 1541) — `total += double(x)` fires when caller pre-casts the projection to double (e.g. `_select(double(_.price)).average()`). Wrap in static_if guarded by `typeinfo stripped_typename(x) == typeinfo stripped_typename(default<double>)` so the cast only emits when needed. 2. **linq_fold.das average splice** (plan_loop_or_count, line 734) — emitted `double(accName) / double(cntName)` unconditionally. accName carries accType, which for double-projected chains is already double. Branch on `accType.baseType == Type.tDouble` to skip the cast. 3. **linq_fold.das count-shortcut emissions** (emit_length_shortcut line 432 and the plan_zip length-shortcut line 3362) — emitted `int(length(...))` for count. length already returns int, so the cast is dead weight. Split into `length(...)` for count and `int64(length(...))` for long_count. No semantic change. Closes the CI lint failure on PR #2760 (5 new + 2 of 4 pre-existing PERF020 warnings in changed files). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
borisbat
added a commit
that referenced
this pull request
May 20, 2026
…ssion PR #2753's new PERF020 rule (`T(x)` where x is already T) caught three real sloppy-codegen sources that all emitted unnecessary workhorse casts the interpreter doesn't fold: 1. **daslib/linq.das average()** (iter + array overloads, lines 1529 / 1541) — `total += double(x)` fires when caller pre-casts the projection to double (e.g. `_select(double(_.price)).average()`). Wrap in static_if guarded by `typeinfo stripped_typename(x) == typeinfo stripped_typename(default<double>)` so the cast only emits when needed. 2. **linq_fold.das average splice** (plan_loop_or_count, line 734) — emitted `double(accName) / double(cntName)` unconditionally. accName carries accType, which for double-projected chains is already double. Branch on `accType.baseType == Type.tDouble` to skip the cast. 3. **linq_fold.das count-shortcut emissions** (emit_length_shortcut line 432 and the plan_zip length-shortcut line 3362) — emitted `int(length(...))` for count. length already returns int, so the cast is dead weight. Split into `length(...)` for count and `int64(length(...))` for long_count. No semantic change. Closes the CI lint failure on PR #2760 (5 new + 2 of 4 pre-existing PERF020 warnings in changed files). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
profelis
pushed a commit
to profelis/daScript
that referenced
this pull request
May 20, 2026
cond ? T(a) : T(b) where both branches apply the same workhorse cast emits two ExprCall nodes that do identical work. Hoist to T(cond ? a : b) — one call instead of two, same evaluation semantics. Suggested in the PR GaijinEntertainment#2753 review by @aleksisch: "string(a ? b : c) instead of a ? string(b) : string(c)? Can be added to linter too btw. It's not first time such code was written." The rule reuses PERF020's 15-name workhorse-cast set (int/int8/int16/int64/uint/uint8/uint16/uint64/float/double/string/ bitfield/bitfield8/bitfield16/bitfield64) and fires when: - Both ternary branches resolve to the same workhorse cast name. - Both calls share the same target Type. - The user argument on both branches has the same baseType — so the hoisted T(cond ? a : b) typechecks without an intermediate cast. Different-arg-baseType cases (cond ? string(intV) : string(int64V)) intentionally do NOT fire — the rewrite would need a manual widen and that is left to the author. The rule fires anywhere, including inside closure bodies, matching PERF020's stance: a redundant cast is redundant regardless of scope. Argument-count gate accepts any >=1 to handle string(int) (bound with explicit args({"value","hex","context","at"}) -> 4 daslang args), which the original single-arg gate would have missed. Drive-by: same-PR daslib sweep -- three perf_lint.das self-hits (call.func.fromGeneric != null ? string(.fromGeneric.name) : string(.name)) hoisted to the PERF021-suggested form. Zero residual PERF021 hits in daslib post-fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
pull Bot
pushed a commit
to forksnd/daScript
that referenced
this pull request
May 23, 2026
PR GaijinEntertainment#2753 (6bf6817, perf_lint PERF020) silently flipped 2530 lines of this file from LF to CRLF — content was identical byte-for-byte, only the line endings changed. Because .gitattributes marks the file as `binary`, GitHub didn't show a text diff in that PR, so the flip slipped through review. Native compilers ignore stray \r, but Emscripten's clang chokes on the CRLF region with `unterminated conditional directive` at ds_parser.hpp (via the include) and `expected identifier` / `expected ';' after enum` in the cpp. Result: the WASM build step in pages.yml has been silently failing on every master deploy since GaijinEntertainment#2753 merged. continue-on-error keeps the workflow green, the staging gate substitutes placeholder.html for index.html, so daslang.io/playground/ has been serving "Runtime rebuild in progress." for the last several deploys. Restored blob hash matches the pre-GaijinEntertainment#2753 version exactly (0d581f1). Line count unchanged (12967); file size −2530 bytes (one \r per CRLF). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds PERF020 to
daslib/perf_lint. FlagsT(x)wherexis already of workhorse typeT— a no-opExprCallagainst the cast-named overload that the typer does not elide.Fires for the 15 workhorse cast names registered by
ADD_NUMERIC_CASTS/ADD_BITFIELD_CASTS+das_lexical_cast_*:int,int8,int16,int64,uint,uint8,uint16,uint64,float,double,string,bitfield,bitfield8,bitfield16,bitfield64.Implementation
call.func.fromGeneric?.name ?? call.func.nameis in the workhorse-cast set ANDarg._type.baseTypeequals the cast's target type. Const/ref/temp qualifiers on the argument are ignored — onlybaseTypematters.preVisitExprCallso the rule fires anywhere (a redundant cast inside a closure body is still redundant).perf_warninghandles macro-generated / generic-instantiation suppression.MyBitfield(x),MyEnum(x)), vector ctors (int2(x,y)etc.), andstring(das_string)(covered by PERF007 / PERF012). The single-argument gate excludes multi-arg ctors; user-named ctors don't match the workhorse name set.Drive-by
daslib/perf_lint.das:394— droppeduint64(intptr(...));intptralready returnsuint64. Self-hit found by the new rule.utils/lint/tests/perf019_int_cast_collapse.das—good_nested_castusedint(int(m1))deliberately to exercise PERF019's nested-call unwrap, but that shape is now a textbook PERF020 hit. Replaced inner cast with anint64widen so the same PERF019 code path is exercised without tripping PERF020.CI
The lint-on-changed-files gate at
extended_checks.yml:225-238already runs the unified lint runner on every changed.dasfile in PRs. No workflow edits needed — the new rule rides the existing pipeline.A daslib sweep found ~30 PERF020 hits in 14 other files (aot_cpp.das, ast_boost.das, coverage.das, debug.das, profiler.das, others). Per the standard rule-then-sweep pattern, those go to a follow-up cleanup PR — they don't fail current PRs unless those files are touched.
Test plan
dastestonutils/lint/tests/perf020_redundant_cast.das— 11 PERF020 fires, matchesexpect 31208:11dasteston allutils/lint/tests/— 28/28 pass after PERF019 fixture restructuredaslib/perf_lint.das(paranoid+perf+style) — PASSdas-fmt --verifyon the two changed.dasfiles — Verifiedlint.rstextended_checksjob will lint changed files; daslib sweep showeddaslib/perf_lint.das(the only daslib file in this PR) is clean🤖 Generated with Claude Code