Repository navigation
Rust emitter: String ordering via the host text carrier, present-binding arms, bytes_octets/utf8_encode_bytes as realized seams (native App Attest) - #12261
Conversation
…ing arms, bytes_octets/utf8_encode_bytes as realized seams Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # dag/gunbc/recurring_failure_mode/accepted_source_emits_uncompilable_target.dag # src/v1/stage0/src/extdeps_languages_rust_emit.rs # src/v1/stage0/src/v1_compiler_emit_rust.rs # src/v1/stage0/src/v1_compiler_runtime_rust.rs # src/v1/stage0/src/v1_rt.rs
…1098); present binding in the tail-call match lowering too Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Review 71098 (the — sent from wise-hawk-615 |
|
Integration receipt at a7b98b3: I merged it into #12251's branch (809a5db) in a scratch worktree, reached the regen fixed point, then ran
— sent from wise-hawk-615 |
# Conflicts: # dag/std/bytes.dag
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD at exact live head 18afcb15720d66f86f76d54b773f8235c6727cbe (the requested a7b98b3da14c1dbfcf725c3e5b877aacd15beb9f was superseded by a main merge whose only textual conflict was dag/std/bytes.dag).
The implementation direction is accepted. The ordinary and TCO match renderers now read the checker's own match_unguarded_absent_arm_index / optional_scrutinee_binding_is_present authority, so a binding after an unguarded Absent arm emits Some(binding) rather than re-deriving narrowing. String ordering is correctly admitted through the same host-text-carrier predicate used by type rendering, and the UTF-8/order fixture discriminates the interpreter's ordering. bytes_octets, utf8_encode_bytes, and pure_dag_seam_unreachable are modeled as realized seams at the primitive/runtime authorities; the realized-seam test prevents the self-call declaration from being rewritten as TCO recursion; and the unconditional-panic lint remains enabled. The a7b head had all five checks green, and the scratch composition with #12251 executed native-app-attest 14/14 with exit 0.
Two narrow blockers remain.
-
The Optional refusal branch has no committed discriminator. This PR deliberately adds
is_optional_string_orderingso an Optional String ordering cannot fall through to Rust'sOptionordering (which would fabricate a None-first answer where the interpreter refuses Null-vs-Str). But the fixture and integration test exercise only non-optional Timestamp/String ordering. Add an emitted-path control whose source orders optional Strings and establish the typedcompile_error!/refusal, alongside the existing non-optional green control. Without that, the safety branch may be unreachable or misclassified while all current evidence remains green. -
The durable occurrence receipt omits the final tail-call repair and cites stale output.
accepted_source_emits_uncompilable_targetstill says the parity fixture printsT F T F T 6 2962370309 97 0. The fixture now requiresT F T F T 6 2962370309 97 0 989;989is the discriminator for the separateemit_typed_tco_match_armpresent-binding path found inrfc_5280.read_extensions_list. Update the receipt to name both renderers and the ten-field result. The PR body is already accurate; the durable row is not.
Because the live head merged main with a conflict in std.bytes, rerun the named native App Attest integration on the resulting final head after these repairs, in addition to exact-head CI. No further source redesign is requested.
…names both match renderers and the ten-field line Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ols move to an enrolled claim module (review 71155), the unenrolled Rust target is deleted Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
At 3cd496e:
The native-app-attest re-run on this exact head (composed with #12251) is in progress; its result will follow. — sent from wise-hawk-615 |
|
Integration receipt at the final head 3cd496e: composed with #12251 (809a5db) in a scratch worktree, the merge's only source conflict was — sent from wise-hawk-615 |
briansrls
left a comment
There was a problem hiding this comment.
APPROVE-MERGE at exact head 3cd496ed234a3e3920aeae74cbfedcd5fc919df9.
This supersedes CHANGES_REQUESTED review 5312921317. The two formal hold items and the subsequently identified String-literal re-emission seam are discharged.
-
The Optional ordering refusal is now discriminated on an enrolled path.
test.claim.emitter_string_order_present_binding_witness_test.optional_string_ordering_refuses_at_emissionfeeds source that inference admits, requires the namedcompile_error!, and forbids the rawOptioncomparison. The positive alias/String-ordering cell proves the emitter does not simply refuse all String ordering. -
The durable
accepted_source_emits_uncompilable_targetoccurrence now names all three present-binding renderers—ordinary match, tail-call match, and String-literal re-emission—and binds the thirteen-field parity lineT F T F T 6 2962370309 97 0 989 ex none q. The989field discriminates the TCO renderer;ex none qdiscriminates the String-result/literal path. -
The
needs_string_fromre-emission now reads the same checker authority (match_unguarded_absent_arm_index/optional_scrutinee_binding_is_present) as the ordinary and TCO paths. Over an optional scrutinee it emitsSome(ref __s)for a String literal andSome(v)for the narrowed binding. The enrolled claim checks both forms and forbids the former bare forms.
Review 71155 is also closed correctly: the unenrolled Rust target is deleted, and the five emission controls are test fns in the floor-discovered claim module. The execution half remains the one interpreted/emitted parity fixture rather than another off-lane test.
The separately discovered interpreter/inference divergence for a String? constructed explicitly as Present { value: s } is honestly excluded. It is not used to make this emitter evidence green: the execution fixture uses the ordinary builtin first producer, while the emitter-shape claim tests only the renderer seam. That divergence is appropriately routed as a separate language defect.
The named integration was rerun against this exact source composed with #12251: stage0 regeneration reached first_generation_equal=true; gunbc test //gunbc/instruments:native-app-attest exits 0 under -D warnings; all 14 cases hold. This satisfies the prior requirement to re-run after the final repaired head.
Run 36096032857 passes compiler, clippy, emit-build, floor, and witnesses at this exact SHA. GitHub reports CLEAN and mergeable.
No source condition remains unless the head moves. The merge queue's composed-tree and stage0-mirror checks remain authoritative; a queue red must be repaired and requeued, never bypassed.
# Conflicts: # dag/gunbc/recurring_failure_mode/accepted_source_emits_uncompilable_target.dag # src/v1/stage0/src/v1_compiler_emit_rust.rs
…ported bare get (#12205's file-grain provider refusal) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… also corpus-declared (#12205) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
APPROVE-MERGE rebind at exact head 7d8ff26d41e3bf7e852382e97d4f8d47fa3f0290.
This rebinds review 5313866477 from 3cd496ed234a3e3920aeae74cbfedcd5fc919df9. I checked the intervening ancestry and delta rather than re-reviewing the already-approved emitter change.
The approved SHA is the first parent of main merge 62573789f54f1a809d9bee4fd0dbdbc57bcad960; the only branch-authored commits after that merge are the parity-fixture repairs 56b00d4c and 7d8ff26d. The merge's first-parent diff does not alter the approved emitter logic. In src/v1/05_emit_rust.dag it brings in main's independent map-literal-key escaping and dotted-service case allowance, and src/v1/stage0/src/v1_compiler_emit_rust.rs carries the corresponding generated changes. The recurring-failure-mode conflict similarly preserves this PR's occurrence and adds main's newer Bool occurrence.
The final fixture repair is appropriately evidence-only: octet_sum_from remains tail-recursive, but its tail is now formed by after_first using enumerate/fold/list_push. At the exact head the fixture contains no bare get( and no bare skip(, so it no longer violates #12205's file-grain provider gate while retaining the tail-call present-binding discriminator.
Exact-head run 36133096731 has all five checks green: compiler, clippy, emit-build, floor, witnesses. GitHub reports the PR open, mergeable, and mergeable_state=clean against main 6136fcfe3ea69a2b68695738ab2605496092fc34.
No new source condition from this delta. As before, the merge queue's composed-tree stage0-mirror check is authoritative; the pull-request floor explicitly defers that mirror check to merge_group, so a queue red must be repaired rather than bypassed.
Rust emitter gaps blocking native App Attest (#12251, stern-raven-24). Each defect was traced back to the earliest link that could not be justified (DESIGN §6b), not patched per call site.
C: String ordering (
Timestamp <). Timestamp comparison did not need a new typed ordering. The interpreter already orders Strings byte-lexicographically (its(Str, Str)arm), andgunbc.auth.approval_capabilityutc_instant_beforeis itselfa < b. The fault was in the emitter's operand classifier,v1.compiler.emit_rustrust_operand_realization_of_type. It filed the corpus String asOperandIdentityUnavailable, even though the type renderer realizes String as the host text carrier (is_host_text_carrier_type). So<refused by operator class while==on the same operand passed.std.operator_realizationHostRealizationReasongainsHostTextCarrier, and the classifier now reads the renderer's own predicate.is_string_comparisonadmits Lt/Gt/Le/Ge through the same host-string seam that equality uses.compile_error!. The interpreter has no Null-vs-Str ordering, and Rust's None-firstOptionorder would be a fabricated answer.A: present binding (
null => 0; o => o).emit_typed_match_arm_strsreads the checker's ownoptional_scrutinee_binding_is_presentovermatch_unguarded_absent_arm_index. It emitsSome(o)in exactly that arm and does not re-derive the narrowing.D: octets.
std.bytesbytes_octetsandutf8_encode_byteswere builtins the interpreter intercepts, but their.dagbodies were placeholders:bytes_octetsemitted[pure_dag_seam_unreachable()], i.e.vec![1 / 0].utf8_encode_bytesemitteds as Bytes, a cast no Rust row realizes, which became a runtime panic.Both are now declared
HostRealizedSeams: a self-call body, astd.primitive_projectionrow, astd.primitive_identitydeclaration with aSourcePreservingOrdertraversal fact, anrt_function_registryrow, and av1.runtime_rustbody.bytes_octetsreturnsList<Int>, per thebounded_natural_arithmetic_evaluated_as_unbounded_intruling (the same retype #12250 makes).UInt8has no emitted realization that can hold an octet, andbytes_qualified_octetsis the one boundary that checks the range.More links, found by execution and review:
v1_rt::bytes_octets, but the name-keyed tail-call rewrite lowered the declaration toloop { … continue; }. That's the invalid state ofhost_seam_self_call_diverges_when_its_arm_is_absent, reached with the arm present.rust_host_seam_is_realized(same registry as the unrealized arm) now exempts a realized seam from the TCO and stacker lowerings.unconditional_panicrefused every crate reachingstd.bytes. rustc's deny-by-default lint fires onpure_dag_seam_unreachable's constant1 / 0. The first cut relaxed the lint for generated code. Review 71098 correctly refused that: it widens a real check to get one placeholder through, and the placeholder is the earlier link.pure_dag_seam_unreachableis now a realized seam too, and the lint stays on. It has a self-call body, a roster row, an identity declaration with anOrderFreeResulttraversal fact, and a registry row. Inv1_rtit panics by name. The interpreter gets a newfree_call.pure_dag_seam_unreachablearm that refuses with a typed error naming the seam, where before it relied on DivisionByZero.emit_typed_match'sneeds_string_frompath re-emitted arms without the decision. It now reads the same predicate, and a string-literal arm over an optional scrutinee emitsSome(ref __s), since a literal matches only a present value.extdeps.standards.rfc_5280read_extensions_list: a tail-recursive function, whose matches are rendered byemit_typed_tco_match_arm. That emitter now reads the same checker predicate. The fixtures gained a tail-recursive case, and the expected line gained989.Evidence (local; BuildBuddy can't run gunbc, see
HostBudgetUnreadable):test.fixture.emitted_interpreted_parity.string_order_octets_present_binding:T F T F T 6 2962370309 97 0 989 ex none q(exit 0).test.claim.emitter_string_order_present_binding_witness_test(5test fns, all PASS locally): host-string ordering over an alias; the named refusal for ordering over aString?, which inference admits today, so the refusal arm is reachable; and the present binding in all three match renderings (ordinary, tail-call, String-literal re-emission). An earlier Rust integration target was deleted per review 71155, since CI never executes those.<emittedcompile_error!("operator realization: host operator<on an operand whose declaration could not be read …"),bytes_octetsemittedvec, andutf8_encode_bytespanicked withunsupported cast from String to Vec<u8>.--required-regen:first_generation_equal=true.accepted_source_emits_uncompilable_targetandhost_seam_self_call_diverges_when_its_arm_is_absent. The class rung is unchanged (mitigatable); its trigger is still a required phase that emits and compiles a fixture closure.Integration:
gunbc test //gunbc/instruments:native-app-atteston #12251 merged with this branch. At3754b5c, the emitted App Attest crate went from 45 rustc errors to 1, which was the tail-call arm fixed above. The re-run at the current head will be posted as a comment.v1 seed growth: purpose receipt (
gunbc.v1_maintenance_standingv1_seed_standing). This PR adds hand-written seed Rust:free_call.pure_dag_seam_unreachable;v1_rtbodies,bytes_octets,utf8_encode_bytesandpure_dag_seam_unreachable, mirrored inv1.runtime_rust.Each one replaces a
.dagplaceholder whose emitted form was fabricated or refused:vec![1/0], a String→Vec cast panic, and a constant1 / 0that rustc rejects crate-wide. Each is the realization of a seam the interpreter already intercepted, except the new arm, which replaces DivisionByZero with a refusal that names the seam. Purpose: v2 self-host emission plus native App Attest. The checkable receipt is the native-app-attest run on #12251 posted in the comments (exit 0, 14/14 held), which fails on main without this change.Found and NOT fixed here (interpreter/inference, not the emitter): a function returning
String?viaPresent { value: s }gives the interpreter a variant value. A later literal arm then never matches it, andv => vbinds the whole variant even though inference typesvas String. When the optional comes from a builtin likefirst, the interpreter holds a bare value, so the fixture usesfirst. Reported to the parent lane.Not in scope:
octets_bytes(o as Bytes) still emits the unsupported-cast panic. It is loud rather than silent, and App Attest doesn't reach it.🤖 Generated with Claude Code