Conversation
norac previously reported failures as bare "Parsing failed!" / "Interpretation failed!" strings (or nullptr sentinels) with no source location. Introduce a DiagnosticEngine wrapping llvm::SourceMgr that renders errors in the familiar file:line:col form with the source line and a caret. SourceStream now registers its buffer with the engine and maps byte offsets to SMLocs; AST nodes carry an SMRange threaded through the parser so diagnostics can point at the offending expression. The parser emits located errors at keyword-form commit points and at the top-level linklet structure, guarding against cascades once an error is reported. The interpreter routes unbound-identifier, arity, non-procedure, set!, let-values and runtime errors through the engine and propagates evaluation failures via a null result instead of asserting. main exits non-zero when the engine reports any error. Adds negative integration tests (not + FileCheck) and DiagnosticEngine unit tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
+ Coverage 75.61% 76.47% +0.85%
==========================================
Files 24 26 +2
Lines 2391 2567 +176
Branches 332 371 +39
==========================================
+ Hits 1808 1963 +155
- Misses 583 604 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 000c9a0224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Identifier's copy constructor, copy assignment and move assignment re-initialised the ASTNode base via ClonableNode(Kind), dropping the SMRange. ClonableNode::clone() restores the range, but SetBang's copy constructor copies its nested Identifier via the copy constructor (new Identifier(*SB.Id)), not clone() — so a set! target inside a lambda body lost its location once the body was cloned into the closure, and the unbound-set! diagnostic printed <unknown>:0 instead of pointing at the identifier. Carry the range across all three Identifier copy/move operations so it survives copy-construction wherever identifiers are stored by value (set! targets, define-values/let-values id lists, lambda formals). Adds a regression test for set! inside a lambda. Addresses review feedback on PR #84.
Resolve conflicts in the interpreter with main's case-lambda / letrec-values refactor: adopt applyFormals()/bindClause() and the Closure/CaseLambdaClosure dispatch, and route their error sites (arity mismatch, case-lambda no-match, let-values value-count) through the DiagnosticEngine with source locations, keeping the diagnostics consistent. Convert the new letrec-values assert(Result) into error propagation.
Keep both the new #%variable-reference parser test and the DiagnosticEngine unit tests; no source conflicts this round.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31421b9cc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Diag.error(A[0].getLoc(), | ||
| "application: expected a procedure in operator position"); |
There was a problem hiding this comment.
When the operator expression is a #%variable-reference, parseVariableReference never assigns a source range to the AST node, so this new A[0].getLoc() path receives an invalid location and reports <unknown>:0 for inputs like (linklet () () ((#%variable-reference) 1)). Either give VariableReference nodes a range when parsing them or fall back to the application range here so the diagnostic remains located.
Useful? React with 👍 / 👎.
Reconcile the CEK abstract machine (with-continuation-mark, #11) with the features main gained since this branch was opened: the DiagnosticEngine error system (#57/#84) and the case-lambda (#3), letrec-values (#8), and #%variable-reference (#13) / keyword forms. - Closures now capture the live lexical environment chain instead of a copy of their free variables. This gives correct lexical scoping and makes letrec-values forward/mutual references resolve, and it subsumes the earlier dynamic-scope fix. The interpreter tracks every scope it creates and clears their bindings at teardown to break the closure/scope shared_ptr cycles this introduces (verified clean under LeakSanitizer). - All interpreter error paths now report through the DiagnosticEngine at the offending node's source location, matching main's messages; main() decides the exit status from Diag.hadError(). - case-lambda / case-lambda closures, #%variable-reference, and keyword are evaluated by the machine (case-lambda selects the first clause whose formals accept the argument count); letrec-values fills a recursive scope in place. All presets green: ctest 21/21, nora-lit 88/88 under debug, asan, and ubsan; warning-free build; clang-format clean.
1 participant
Summary
Replaces norac's ad-hoc error reporting (bare
Parsing failed!/Interpretation failed!strings andnullptrsentinels with no sourcelocation) with a diagnostic layer built on
llvm::SourceMgr, renderingerrors in the familiar
file:line:col: error: …form with the source lineand a caret.
nora::DiagnosticEngine(src/Diagnostics.{h,cpp}) wrapsllvm::SourceMgr,tracks error/warning counts, and emits located and location-less diagnostics.
Uses the already-linked
Supportcomponent — no CMake/link changes needed.SourceStreamregisters its buffer with the engine andmaps byte offsets to
SMLocs; everyASTNodecarries anSMRangethreadedthrough the parser (and preserved across
clone()), so diagnostics point atthe offending expression.
(
lambda,if,set!,begin,define-values,let-values,quote,values,vector) and across the top-level linklet structure, withhadErrorguards so a single error unwinds cleanly instead of cascading.Pure backtracking on valid input is unchanged — no diagnostic ever fires on a
well-formed program.
set!onunbound,
let-values/define-valuescount, and runtime errors through theengine at the relevant node's location, and propagates evaluation failures via
a null result (replacing
assert(Result)sites that would abort on error).SourceStreamandInterpreter, andexits non-zero whenever any diagnostic is reported.
Out of scope (left as internal, non-user-facing): the
Values::writeandAnalysisFreeVarsinvariant guards, and lexer-levelINVALID-token recovery.Closes #57
Test plan
ctest --preset debug— 16/16 pass (unit + lit)not norac … | FileCheck): unbound id,arity, non-callable, malformed lambda,
+type error, non-linklet,missing
ifelse,set!unboundDiagnosticEngine/getLocCatch2 unit tests-DCMAKE_COMPILE_WARNING_AS_ERROR=ONclang-format-22clean