Mono poly morphic - #2
Conversation
There was a problem hiding this comment.
RuboCop found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2 +/- ##
===========================================
+ Coverage 91.77% 91.94% +0.17%
===========================================
Files 507 619 +112
Lines 160551 260853 +100302
Branches 20024 20685 +661
===========================================
+ Hits 147343 239847 +92504
- Misses 10076 17696 +7620
- Partials 3132 3310 +178
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
| Project | CLEAR |
| Branch | mono-poly-morphic |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | leak-build-ms | Measure (units) x 1e3 | leak-count | Measure (units) | leak-run-ms | Measure (units) |
|---|---|---|---|---|---|---|
| benchmarks/concurrent/01_socket_throughput/bench | 📈 view plot | 2.60 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 7.73 units |
| benchmarks/concurrent/06_dynamic_spawn/bench | 📈 view plot | 2.47 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 38.59 units |
| benchmarks/concurrent/11_parallel_aggregation/bench | 📈 view plot | 2.64 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 16.28 units |
| benchmarks/concurrent/18_atomic_counter/bench | 📈 view plot | 2.56 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 5.69 units |
| benchmarks/inter-clear/04_concurrent_mvcc_fat_struct/bench | 📈 view plot | 2.79 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 167.64 units |
| benchmarks/sequential/03_alloc_throughput/bench | 📈 view plot | 2.32 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 7.37 units |
| benchmarks/sequential/08_sort/bench | 📈 view plot | 2.52 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 5.09 units |
| benchmarks/sequential/13_soa_layout/bench | 📈 view plot | 2.48 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 738.70 units |
| benchmarks/server/03_pathological/server | 📈 view plot | 2.75 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 1,002.84 units |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
| def with_conditional_context(&blk) | ||
| fn_ctx = current_fn_ctx | ||
| enter_conditional_context!(fn_ctx) | ||
| blk.call |
| def with_loop_context(&blk) | ||
| fn_ctx = current_fn_ctx | ||
| enter_loop_context!(fn_ctx) | ||
| blk.call |
| end | ||
| def self.resolve_require_targets(target, member_dir, resolve_pkg) | ||
| if target.start_with?("pkg:") | ||
| resolved = resolve_pkg.call(target.delete_prefix("pkg:")) |
| def with_kept_edge_call_frame(&blk) | ||
| T.bind(self, MIRLowering) | ||
| kept_edge_temp_stack << KeptEdgeCallFrame.new | ||
| args_mir = blk.call |
| arg_type = kept_edge_arg_type(arg) | ||
| conflict = kept_family_conflict(arg_type, family) | ||
| if conflict | ||
| on_family_violation&.call(arg, conflict, family, site.callee_name) |
| next unless type_info.generic_instance? | ||
|
|
||
| schema = schema_lookup.call(type_info.resolved) | ||
| schema = schema_lookup.call(type_info.generic_base) if schema.nil? |
| item1 = NEXT out; | ||
| item2 = NEXT out; | ||
| MUTABLE total = 0_i64; | ||
| total = total + #{item_observe.call("item1")}; |
| item2 = NEXT out; | ||
| MUTABLE total = 0_i64; | ||
| total = total + #{item_observe.call("item1")}; | ||
| total = total + #{item_observe.call("item2")}; |
| <<~CHT | ||
| MUTABLE total = 0_i64; | ||
| WHILE NEXT out EXISTS AS item DO | ||
| total = total + #{item_observe.call("item")}; |
| @@ -3629,6 +3778,14 @@ | |||
| return self.shards[s].map.get(key); | |||
| } | |||
|
|
|||
| pub fn getPtr(self: *Self, key: K) ?*V { | |||
| const s = shardIndex(key); | |||
| const elided = self.locks_elided.load(.monotonic); | |||
There was a problem hiding this comment.
Nil-Kill found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Decomplex found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Espalier found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Everything from self-host-i except the generated self-hosted tree (compiler/src, compiler/.src-transpiled), the ruby-to-clear transpiler (gems/ruby-to-clear), and the fact-mine/lineage gem work (carried separately on fact-mine-aliasing-exp), so this can land on master while self-host-i stays unmerged. Compiler fixes (each with a regression test): - Type#needs_cleanup? made non-owning strings owning under a wrapper. Symbols are interned and raw strings borrowed, so ?String@symbol / !String@symbol must not need cleanup; the classifier then asked for a DROP the lifecycle plan refused and lowering aborted. Both wrapper branches now use owning_string?. - visit_GetIndex crashed with an internal RuntimeError when indexing a struct with no element type instead of reporting UNSUPPORTED_INDEX. - resolve_inherent_static_call! left the resolved return type unstamped. - A lazy require of WithMatchCheck used a wrong path, masked by load order. - Removed the dead HasExpression module: all 22 includers are Structs whose generated reader shadows it, and it broke AOT compilation. Tooling: - tools/sorbet_strip.rb mirrors compiler/ruby to compiler/.ruby-rbs with Sorbet removed plus RBS sig export; 2.2x faster builds (203s -> 93s on the self-host parser), verified by byte-identical Zig on 5452 programs (tools/zig_equivalence.rb). - tools/selfhost_build.sh builds against that mirror and regenerates it when compiler/ruby is newer -- a stale mirror silently rebuilds pre-fix sources. - Incremental compilation now fingerprints tokens outside function bodies rather than raw source, so a comment edit no longer forces a full rebuild (15.8s -> 2.7s on vm.clear). - parser_compat.rb reads both REQUIRE spellings, so cyclic module clusters are found and merged into multi-file packages instead of reporting a circular dependency. Also: 4 fuzz cells covering non-owning strings under a wrapper (an axis the corpus never composed), and a .gitignore rule that was silently ignoring the self-hosted parser sources. CI follow-ups on this branch: - Sorbet EnforceSignatures: four attr_readers in type.rb schema classes had no sig; typed from their T.let declarations rather than autocorrected to T.untyped. - sorbet/config now ignores compiler/.ruby-rbs, the generated stripped mirror. It is gitignored so CI never saw it, but locally it collided with compiler/ruby and produced 9488 phantom errors. - Three benchmarks still used the retired `~?T[]` stream syntax, which this branch's parser rejects; converted to `[~]T` / `[~]@split T`. - IF-EXISTS captures: 119dc95 excluded node_reference? from the mutable-slot-payload rule, so `IF nodes[i] EXISTS AS n THEN n.f = ...` failed as an immutable-field assignment. A @node handle is a pointer into the NodeStore, so assigning through the capture lands in the stored node; the exclusion is now Rc-only. Regression test added. - Two corpus sources needed migrating to the new carrier rules (rule 6, added deliberately by 9f39b56): mal's envSet fans `val` out to a map store and a recursive call, so it takes MONOMORPHIC and KEEPs at the fan-out; the kvstore benchmark assigned a plain local across three exclusive arms with no ELSE, so it uses COPY. Sorbet type-check: master is clean, this branch had 237 errors. Fixed: - 153 redundant T.cast/T.must the branch's sharper types made useless, plus the multi-line cases Sorbet's own autocorrect removes. - T.bind(self, ...) added to the module methods that call into their includer, matching each file's existing idiom (the annotator domains bind without `rescue nil`, which otherwise makes Sorbet ignore the bind). - escape_analysis#matches? takes a BasicObject, so its is_a? checks need T.unsafe, as master does. - protocol_projection_resolver#issue takes **values; a kwargs splat is typed by its value type, not as a Hash. - dotted_type_value_zig_name called module_eval(namespace), which is Ruby's Module#module_eval and does not exist on the instance -- it would raise at runtime. Restored master's "#{namespace}.#{node.field}". - Loop-reassigned locals declared with T.let; DiagnosticRegistry.format_from_hash can return nil; LSP diagnostics templates are only Strings; mod_alias normalized to String-or-nil; make_rc_retain guarded on Identifier. - sorbet/config ignores compiler/.ruby-rbs and scratch/, both generated or local and gitignored. scratch/struct_flow.rb defines a conflicting `Token = Struct.new(:type, :value)`, which made Sorbet believe Token#initialize took 0..2 args -- that is what led `srb tc -a` to silently delete arguments from Token.new/ParserError.new call sites. - sorbet/rbi/clear-attr-accessors.rbi regenerated; CI diffs it against the generator and the committed copy was 35 lines stale. Architecture SARIF: the job hung until its 45-minute deadline on every branch, holding a runner and blocking its concurrency group, so queued jobs on other PRs starved behind it. fact-mine's call-resolution pass was being handed gems/lineage/src/ui/assets/diff/assets/index--_D05yx7.js -- a 222 KB Vite bundle, 61 lines of minified JavaScript. corpus_common already skips *.min.js and dist/build/node_modules, but these bundles sit under .../src/ui/assets/, which nothing matched. Excluded by path prefix rather than adding `assets` to EXCLUDE_DIRS, so hand-written assets elsewhere stay in the corpus; the three other assets/ directories are already covered by `docs`. The gigasail path is the same tree after the lineage -> gigasail rename, which is why gigasail-master hangs too -- that branch moved the bundles, it did not exclude them. Corpus drops from 723 to 721 files (exactly the two bundles). The job's three scripts now finish in ~4.5 min instead of timing out: cycle_report 136s, reach_through 133s, change_coupling 1s, each emitting valid SARIF. Gates run locally: 7877 specs, 664 transpile-tests (0 leaks), buildable corpus, benchmark transpile coverage shards 1 and 2, Rubocop EnforceSignatures, RBI freshness, and srb tc clean on a pristine checkout. Co-authored-by: Codex <codex@openai.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
`IF map[key] EXISTS AS slot` borrows the slot instead of copying an
ownership-bearing payload, and lowering emits `.getPtr` for that borrow.
Only StringMap implemented it. Map shape is erased whenever the receiver
is a plain `{K}V` parameter -- a `{String}@Sharded(4):writeLocked Node`
argument reaches the callee looking plain -- so the lowering-side shape
guard never fires there and the erased instantiation failed Zig
compilation with "no field or member function named 'getPtr'".
Fix it where shape is actually known: at the comptime instantiation.
Lock-based shapes (ShardedStringMap, MutexShardedStringMap,
ShardedNumericMap, StripedNumericMap, and the StripedStringMap alias)
now answer getPtr under their shard lock. Shared-nothing partitioned
shapes cannot -- each shard is owned by a scheduler and reached through
sendAndWait -- so they answer with a @CompileError naming the two fixes
instead of Zig's undecipherable missing-member error.
The borrow aliases shard storage after the shard lock is released. That
is the window `get` already hands out for aggregate values, widened to
include rehash; the contract note above ShardedStringMap says so.
Broke on this branch when the pointer capture was widened from union
payloads to struct and collection payloads (665), which is what pulled
examples/graphdb's `@sharded(4):writeLocked` map onto the borrow path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
A tuple has no schema, so unlike a named struct it gets no generated __clear_drop and a :uniform cleanup entry hands it to CheatLib.cleanup's structural walk, which frees every []const u8 field. That walk is only sound while every slice element is owned. Symbols are interned rodata and are never duped into owned storage, so a tuple mixing a cleanup-bearing element with a symbol element aborted at scope exit with "incorrect alignment" -- free() on a rodata pointer. Named structs never hit this because their __clear_drop touches only the fields that own something. classify_structural_product already skips symbol and borrowed elements, but it sits at the end of the classifier chain and an earlier :uniform producer wins first. Consult it up front for exactly the tuples the structural walk cannot handle. Tuples whose slice elements are all owned keep their existing uniform cleanup. Covered by extern_resource_raii_integration_spec, which caught this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
`entries |> CONCURRENT SELECT ... |> REDUCE` aborted at scope exit with "Allocation alignment 16 does not match free alignment 8" plus a double free. The concurrent builder allocates the result with pipeline_result_alloc -- the frame allocator here -- but the source binding's cleanup used __clear_heap_alloc, so the frame block was freed through the heap allocator (INV-1: single allocator per binding, fixed at declaration). pipeline_result_heap? reported the annotator's :heap storage stamp alone. The stamp is the placement request; it does not by itself move the builder's allocation, so it only describes where the result landed when the builder also used the heap allocator. Report the conjunction. The fixture spec asserted the heap-allocator cleanup that crashes at runtime, so it now asserts the identity that has to hold: the cleanup allocator matches the allocator concurrentListSelect was handed. Covered by examples/parallel_du/du.clear, which caught this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
Every SARIF job failed at github/codeql-action/upload-sarif with "Resource not accessible by integration" against actions/workflow-runs#get-a-workflow-run, while the analysis step above it succeeded. The action reads the workflow run it is uploading for, which needs actions:read; the jobs granted only contents:read and security-events:write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
Follow-up to 3e9a750, which fixed the du.clear crash in the wrong place. Dropping the :heap claim from the sink allocator stopped the alignment mismatch but left 12 pipeline-matrix cells leaking, because that same claim is what supplies the allocator the result is freed through -- withholding it withheld the cleanup too. The mismatch is at the allocation, not the cleanup. pipeline_alloc is the pipeline's placement decision and already honours the annotator's storage stamp (INV-16), and complex_pipeline_sink_alloc frees through it; but the concurrent builders construct their result with pipeline_result_alloc, which ignores the stamp. A :heap-stamped pipeline therefore allocated in the frame and freed on the heap. Route the concurrent builders through the same decision, so the stamp reaches the allocation it was describing and one allocator owns the binding end to end (INV-1). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
`clear test` documents that CLEAR programs resolve relative paths from the user's invocation directory, and the plain path preserves that with a --test-cmd wrapper. The coverage path ran the binary from the source file's directory instead, so the same test read different files with and without --coverage. The two corpus tests were written against the coverage behaviour, reaching back out with "../../examples/..." -- which encodes the build layout and fails whenever the run honours the documented contract. They are the only CLEAR sources in the tree that do this. Run the instrumented binary from the invocation directory too, and point both fixtures at repo-relative paths. run_zig_test also honours run_dir on its uninstrumented path now, so the cwd no longer depends on whether ZIG_COVERAGE is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkBJZMTAuVZCVrghaLWXEh
2236e02 to
307bf76
Compare
| # (default, carrier-preserving), :unique (exclusively owned), :shared | ||
| # (requires a retained-identity family). | ||
| contract = T.let(self[:carrier_contract], T.nilable(Symbol)) | ||
| self[:carrier_contract] = contract || :polymorphic |
| actual_key = args[0].full_type!(context: "HashMap.put key") | ||
| expected_key = obj_type.key_type | ||
| unless obj_type.accepts_map_key?(actual_key) | ||
| error_fn.call(node, "HashMap.put: key must be #{Type.surface_name(expected_key)}, got #{Type.surface_name(actual_key)}") |
| actual_key = args[0].full_type!(context: "HashMap.delete key") | ||
| expected_key = obj_type.key_type | ||
| unless obj_type.accepts_map_key?(actual_key) | ||
| error_fn.call(node, "HashMap.delete: key must be #{Type.surface_name(expected_key)}, got #{Type.surface_name(actual_key)}") |
| actual_key = args[0].full_type!(context: "HashMap.contains? key") | ||
| expected_key = obj_type.key_type | ||
| unless obj_type.accepts_map_key?(actual_key) | ||
| error_fn.call(node, "HashMap.contains?: key must be #{Type.surface_name(expected_key)}, got #{Type.surface_name(actual_key)}") |
| end | ||
| }, | ||
| with_fiber_rt: ->(rt_name, blk) { | ||
| with_fiber_capture_map({}, rt_override: rt_name) { blk.call } |
`clear test` pointed --cache-dir and --global-cache-dir inside its per-process ".build-<pid>" directory, which it deletes on the way out. The cache never survived, so every invocation paid a full cold Zig compile -- and the zig/.clear-cache that CI restores was useless for the test path. Zig's cache is content-addressed, so a single shared location is safe. Every test program imports the same runtime/ and lib/ modules, so with the cache shared only the generated root module is analysed per run. The build dir itself stays per-process: it holds the generated ._clear_tmp_<base>.zig, and concurrent tests sharing one would read each other's source. Measured on distinct one-line programs: before 4.7s, 4.9s, 4.9s, 4.9s after 5.0s, 1.9s, 1.8s, 1.8s Also set WORKERS="$(nproc)" on both prspec jobs. parallel_rspec defaults to 4 workers when WORKERS is unset (Workers.number_of_workers), not one per core -- an earlier comment in this file claiming 32 workers was wrong, and is corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both matrix specs carried a second `:integration` describe that shelled out `clear test` once per cell behind an 8-thread pool. Between them that was 1545 of the integration job's ~1750 core-seconds, in TWO rspec examples that no file-level sharding can split. The ownership lane bought nothing for its 380 core-seconds: its cells emit `TEST Surface DO t(); END` with no `main`, and `clear test` does not execute TEST blocks -- a file whose only content is `TEST Surface DO ASSERT FALSE ... END` reports "All 1 tests passed". That is why RUNTIME_KNOWN_FAILURES was empty. Its surface is a strict subset of the fuzz `ownership_surface_smoke` entry (18 value shapes x 8 escape sinks vs 9 kinds x 7 ops) plus the per-sink truthful owners, whose cells are `FN main` programs that do run. The pipeline lane did run, and one of its axes was genuinely uncovered: every fuzz pipeline template varies the operator, source shape, or element expression while pinning where the result LANDS. pipeline_consumer_position_matrix inverts that -- two representative operators (owned SELECT, borrowing WHERE) across the landing sites that decide loop rewind and escape promotion for a pipeline temporary: bound inside FOR/WHILE, bound in a MATCH arm inside a loop, iterated under IF, used as an IF condition, re-piped mid-chain, and escaped into TAKES / a struct field / an outer list. Plus owned REDUCE and join accumulators rebuilt per loop iteration. 26 cells, ~2s each. The compile lanes are untouched: 522 in-process transpile examples that already run in the unit job in about a second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ocess COVERAGE=1 was the integration job's dominant cost, not the specs. These specs shell out to `./clear` hundreds of times and the CLI's coverage_bootstrap boots SimpleCov and writes a resultset in every one of those subprocesses. Same files, same parallelism, measured: fixable_int_overflow_spec 1.1s -> 45.4s (ZERO examples) clear_cli_stack_check_require 1.5s -> 115.6s (1 example) c_ffi_integration_spec 3.1s -> 230.7s (1 example) Dropping it costs the integration path's contribution to the Ruby flag on Codecov. The unit job, transpile-tests, and the bc-lower shards still report Ruby coverage, and none of them pay this tax because they do not fan out through the CLI. With that gone the suite is 340 core-seconds; four file-sharded jobs put the worst shard at 73s on two pinned cores, all 206 examples passing. The 5-minute timeout is a hang guard, not the target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eb89df7 to
604adf8
Compare
A `T[]` binding whose items are all literals lowers to a comptime
`[2][]const u8{ "a:b", "c" }` -- static slices in .rodata, no allocator
involved. classify_binding still handed it a uniform cleanup, so the defer
passed .rodata addresses to the frame allocator.
The frame arena's free is a no-op, which is why every existing test missed
it: `clear run`, `clear test`, and the deleted pipeline_position runtime lane
all reported clean. Under the ZIG_COVERAGE fuzz bundle the cells run against
an allocator that memsets freed bytes, and the same program segfaults in
compiler_rt memset via Allocator.free.
Fixed next to the existing rodata/borrow provenance guards in
classify_binding, which is the same fact one level out: a non-escaping
binding over rodata owns nothing. Gated on !heap_storage, so a literal list
that escapes still materializes and still cleans up.
Found by pipeline_consumer_position_matrix, whose cells pass `["a:b", "c"]`
into `FN f(xs: String[])`.
Not fixed here, and pre-existing: a MIXED literal list such as
`[dup("a"), "b"]` emits `[2][]const u8{ __tmp_2, "b" }` under one uniform
cleanup, which frees the owned temp correctly and then frees the literal.
Making that sound means materializing literal items into the container's
allocator -- real runtime cost on a shape that is currently silent, so it
wants its own decision rather than a drive-by.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diff Coverage BucketsDiff base:
Src Ruby Visibility BreakdownScope: Files: 216 Counts are nonblank, non-comment Ruby source lines. Protected methods are grouped into
Zig Special Coverage Alerts
|
There was a problem hiding this comment.
Src Type Guardrails found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
No description provided.