Make fundamental types part of the language#148
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: zane-lang/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe specification now defines fundamental types and coercion sites more explicitly, treats ChangesLanguage model and specification semantics
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/functions.md`:
- Around line 190-191: Add a blank line immediately after each affected heading:
“3.5 Block-bodied verbs return explicitly” in spec/functions.md (lines 190-191),
“3.6 Handle-typed dynamic reference types have fixed footprint” in
spec/memory.md (lines 298-299), “2.2 Unit abort type” in spec/error-handling.md
(lines 32-33), and “3.4 Unit primary returns are values” in
spec/error-handling.md (lines 109-110); leave the following prose unchanged.
In `@spec/packages.md`:
- Line 17: Update both “No implicit packages” entries in spec/packages.md to
scope the rule to source packages, and explicitly note that the bundled core
package is exempt from the import/manifest requirement and may provide
predeclared fundamental names.
In `@spec/syntax.md`:
- Around line 131-132: Insert a blank line immediately after the new heading in
spec/syntax.md at lines 131-132, and likewise after the heading in spec/types.md
at lines 87-88; make no other content changes.
In `@spec/types.md`:
- Around line 405-414: Update the coercion-site list in the specification to
explicitly include positional arguments of named-constructor calls such as
Vector2.diagonal(...). State that these arguments use the constructor’s declared
parameter types and receive the same implicit conversions as positional
constructor calls, preserving consistent behavior across constructor forms.
In `@stories/error-handling.md`:
- Around line 54-61: Update the earlier error-handling paragraph describing
payload-free failures to remove the stale Void and bare abort contract. Align it
with the Unit-based behavior established in the “The empty door still carries a
value” section: failures carry Unit(), use explicit abort Unit(), and preserve
the corresponding handler binding semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c5e6054b-2d6f-49f0-bc95-75631b622058
📒 Files selected for processing (19)
README.mdspec/control-flow.mdspec/dependencies.mdspec/error-handling.mdspec/functions.mdspec/generics.mdspec/glossary.mdspec/lexical.mdspec/lifetimes.mdspec/memory.mdspec/operators.mdspec/packages.mdspec/syntax.mdspec/types.mdstories/control-flow.mdstories/dependencies.mdstories/error-handling.mdstories/functions.mdstories/types.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
spec/types.md (2)
430-438: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDefine core constructor visibility at every coercion site.
The resolution algorithm only collects visible implicit constructors, but
coreis explicitly unavailable to source imports and qualification at Lines 89-90. That leaves literal coercion underspecified:@concepts$Number→Intand similar conversions may not be visible from ordinary source packages despite Line 438 requiring them. State that bundledcoreimplicit constructors are automatically applicable at all coercion sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/types.md` around lines 430 - 438, Update the coercion-resolution specification around the bundled core constructors to state that they are automatically visible and applicable at every coercion site, despite core being unavailable to source imports or qualification. Preserve the existing unique-constructor, ambiguity, and no-constructor outcomes while ensuring concept-to-fundamental conversions such as `@concepts`$Number to Int are defined for ordinary source packages.
95-109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSpecify that Unit erasure preserves evaluation.
Because
Unitis permitted in fields, arrays, and constructor results, expressions producing Unit may still have side effects. Line 109 should clarify that erasing Unit storage never elides those expressions or changes their evaluation order; otherwise this optimization can change program behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/types.md` around lines 95 - 109, Update the Unit erasure statement in the Unit type section to explicitly preserve evaluation of expressions producing Unit and their original evaluation order, including Unit fields, array elements, and constructor results. Clarify that only runtime storage may be removed; side effects and observable evaluation must remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/packages.md`:
- Line 17: Update the “No implicit source packages” statements at all referenced
locations to limit the import requirement to access from other source packages.
Clarify that a file’s own package is available through its package declaration
and its members may remain unqualified, while preserving the distinction from
the bundled core implementation.
---
Outside diff comments:
In `@spec/types.md`:
- Around line 430-438: Update the coercion-resolution specification around the
bundled core constructors to state that they are automatically visible and
applicable at every coercion site, despite core being unavailable to source
imports or qualification. Preserve the existing unique-constructor, ambiguity,
and no-constructor outcomes while ensuring concept-to-fundamental conversions
such as `@concepts`$Number to Int are defined for ordinary source packages.
- Around line 95-109: Update the Unit erasure statement in the Unit type section
to explicitly preserve evaluation of expressions producing Unit and their
original evaluation order, including Unit fields, array elements, and
constructor results. Clarify that only runtime storage may be removed; side
effects and observable evaluation must remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa37dba8-b73d-4f1d-b9a7-0d833bdd93b1
📒 Files selected for processing (7)
spec/error-handling.mdspec/functions.mdspec/memory.mdspec/packages.mdspec/syntax.mdspec/types.mdstories/error-handling.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
What changed
Int,Float,Bool,String, andUnitunqualified fundamental language types backed by the compiler's bundledcoreimplementation packageVoidwith the real zero-sized singleton typeUnit, usable in storage and generic argumentsreturn Unit(),abort Unit(), andresolve Unit(); handlers always bind their abort value, so no return or error-handling AST node needs aUnitexceptionif/elif/guardconditions coercion sites targetingBool, and counted-loop bounds coercion sites targetingInt@primitives$storageWhy
The original
Voidgeneric-use question exposed two separate inconsistencies. First, an ordinary optionalcorepackage could not supply the stableBoolandIntidentities already required by control flow. Second, makingVoida value while retaining implicit completion would force return-path analysis and AST lowering to recognize one nominal type specially.The specification now keeps
coreas a compiler-bundled implementation package, exposes its fundamental types as predeclared semantic names, calls the singleton typeUnit, and treats its constructor like any other constructor.ifstill namesBoolbecause condition checking inherently has an expected type; returns, aborts, and resolves always carry an explicit expression regardless of type.Validation
git diff --checkCLAUDE.mdVoid, ordinary-core, bare-exit, binderless-handler, and fundamental-home-package rulesSummary by CodeRabbit
Unit) and clarified their value/runtime role and thecorerelationship.if/elif/guardconditions and counted-loop bounds, including a step-by-step implicit-constructor selection algorithm.Unit()values and updated?handler syntax to require binders.zane-version/implicit-package wording.Unitand coercion model.