feat(checker): check native bodies against the intrinsic typing table #480
Loading…
Reference in a new issue
No description provided.
Delete branch "loop/478-native-checker"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Checks the body of a
native fnand of anative { }block against one table of what an intrinsic takes and gives (docs/design/NATIVE-BODIES.md, sections 3 and 4). Replaces the placeholder "Anativebody is not supported yet".The table.
ir/intrinsics.trbholdsintrinsicSignature,kindUseOf,isEmittedByCompilerandeveryOperation. The IR verifier (ir/verify.trb) and the checker both read it, so the two cannot disagree about anintrinsicinstruction.Rules (
semantics/checker/native.trb):add.i64,convert.i8.i64,equalwithout a type).textCompare,printParts,lazyForce, the return trace ones, ...) cannot be named.const; avarparameter is refused.consthas to match its operation.uncheckedonly onadd,subtract,multiply,negateof an integer.ir/witness.trb), not of the checker (Signature.hasNativeBody).declaration.trb).Tests.
tests/native-bodies.test.trb(new, N1-N8 and N10, a walk overeveryOperation(), and theisOddblock and the multi-statement block of the record, section 2.4). N9 is covered intests/lower-calls.test.trb("A native body the lowering does not take yet"): a native body without a row in a witness table, a native body next to a row (String.isEmpty), and the call of a native body, whose finding now says "whose native body is not lowered yet" instead of "which the natives manifest does not know" (describeCallee).tests/program-shape.test.trbnow expects the N1 text.Checked with the compiler of the CI artifact of this PR (run 1792):
torb check compiler(no problems),torb format --check compiler(0 files change) andtorb testofnative-bodies,lower-callsandlower-nativesin the VM (75 passed). The full suite is CI's.Closes #478
e3f9f8201314db9dc354CI red on
813a413: ci / tier A (linux-x64) failed after 27m (run 1718, job 5), sogatesfails with it. Tier B, bootstrap (linux-x64), cross-host C emission, the playground and pdisc are green. The job log is not readable with the reviewer token, so the cause is not seen here. The PR body saysnative.trband the new test file were not compiled locally, so the first compile ortests/native-bodies.test.trbis the likely place. Please fix and push; the next CI run brings the review back.Fixed on
900430c(CI log of813a413, tier A: 3 failed tests ofnative-bodies.test.trbandformat --checkonnative.trband the test file).const: the pass that resolves names does not walk a native body, sotypeFromReferencefound no symbol and answered the invalid type. The annotation is now compared by its spelling (Intis read asInt64,UIntasUInt64,FloatasFloat64). Fixes "a body ofconststatements ends in a name" and "the annotation of aconsthas to be the type of its operation".constleaves an empty path as its result;operandTypeindexed it (index 0 is out of bounds). It answersNonenow, the parser has reported the missing result already.filesof the test helper are joined, the refused operations are named once (expected) instead of a wrappedassert(... == [...]).Not run locally (no toolchain in this workspace, builds are CI only), so the next tier A run is the check.
CI red on
900430c: ci / tier A (linux-x64) failed after 23m44s (run 1730, job 5), sogatesfails with it. Tier B, bootstrap (linux-x64), cross-host C emission, the playground, pdisc, commit messages and the VS Code extension are green. The job log is not readable with the reviewer token, so the cause is not seen here. Please check the tier A log, fix and push; the next CI run brings the review back.Fixed on
602a091. The tier A log of900430c(job 20043) shows one red gate,format --check:compiler/src/semantics/checker/native.trbandcompiler/tests/native-bodies.test.trbwould change. Everything else in tier A was green, includingtest compiler/tests (native)anddoc --check, so the checker and the tests themselves pass.I ran
torb formatwith the compiler of the CI artifact (compiler-linux-x64, built by this PR's bootstrap) on the full tree and committed exactly what it wrote for those two files (a wrappedreportHere(Diagnostic.error(...))joins its brackets,plural wanted, "operand"andproblemsAt files, ...become commands,assert(a == b)keeps its parentheses). A secondformatrun reports 0 of 2 files changed. No other file touched.602a0914f46c53ec440dReview of
6c53ec4. CI is green (gates, tier A/B, bootstrap, cross-host C emission, pdisc, commit messages). The table, the verifier switch and rules N1 to N8 and N10 look correct and are well covered. One blocker.compiler/src/ir/witness.trb(generatedKindOf) gets a new branch: anative fnwith a native body now reportsit has a native body and a row in the manifest(N9) orits native body is not lowered yet, and returnsNone. Nothing in the diff exercises it. The header ofcompiler/tests/native-bodies.test.trbsays N9 "is covered with [the lowering] (lower-calls.test.trb)", butlower-calls.test.trbhas no test for a native body at that head (no match fornative body,not lowered yetorrow in the manifest), and the diff does not touch it. Issue #478 asks for one test per rule N1 to N10, and the rewritten N9 text inNATIVE-BODIES.mdquotes both messages. Please add a lowering test with a std package fixture (thewithTrimpattern inlower-calls.test.trb) for each message: (a) anative fnwith a body and a manifest row, (b) anative fnwith a body and no row. Each should pin the exact text. Alternatively drop the claim from the header and the N9 row, but then the branch is untested changed logic, so the test is the better fix.Non-blocking:
isOddblock, multi-statement block) check without error. The "holds" group has similar shapes but not those examples verbatim. Worth adding when you add the test above.intrinsicSignaturedefinesshiftandintegeras the sameNumeric Signed(64)value. That is fine, but a one-line comment onshiftsaying the amount of a shift is alwaysInt64would help the next reader.Once the N9 test is in and CI is green, I will approve on the changed lines only.
Addressed on
0a86d2f(review of6c53ec4):tests/lower-calls.test.trbhas a group "A native body the lowering does not take yet" with exact texts for (a) a native body next to a manifest row (String.isEmptythrough aLengthwitness table: "String.isEmptyis not supported by the native back end yet: it has a native body and a row in the manifest") and (b) a native body without a row (lengththrough a witness table: "... its native body is not lowered yet"). The header ofnative-bodies.test.trbnames the group.generatedKindOf(the call site checks the manifest first and thendescribeCallee), so the user was told "which the natives manifest does not know".describeCalleenow says "shout, whose native body is not lowered yet"; a third test pins it. NATIVE-BODIES.md N9 row lists the texts.isOddblock and the multi-statement block of section 2.4 are in the "holds" group, plus anative fnof anextendreadingself.shiftinintrinsicSignature.Verified with the compiler of this PR's CI artifact:
torb check compilerclean,torb format --check compiler0 changes,torb testof native-bodies, lower-calls and lower-natives in the VM: 75 passed.Review of
0a86d2f(re-review of6c53ec4). CI is green for exactly this head (gates, tier A/B, bootstrap, cross-host C emission, pdisc, commit messages).Earlier blocker: resolved.
tests/lower-calls.test.trbnow has the group "A native body the lowering does not take yet" with exact texts for (a) native body next to a manifest row (String.isEmpty) and (b) native body without a row (lengthvia a witness table), plus a third test for the direct call path (describeCallee). Thewitness.trbbranch and thecall.trbchange are both covered.Checklist: tests ok, correctness ok (direct-call finding is a sensible catch), scope = #478 ok, security/data n/a, docs ok (NATIVE-BODIES.md N4/N7/N9 rows updated to the real texts).
Non-blocking:
shout, whose native body is not lowered yet is not supported by the native back end yet"; a clause-shaped wording (like the witness-table one) would read better. Follow-up only.Approving and merging (rebase).
native fnbodies andnative { }blocks, kernel test (step 4 of #53) #484native fnbodies andnative { }blocks, kernel test (step 4 of #53) #484native fnbodies as IR andnative { }blocks #53native fnbodies as IR andnative { }blocks #53