Marge bug fixes back to main #2
Conversation
WalkthroughThe PiccodeScript grammar was updated to relax argument default value restrictions, adjust expression parsing order, simplify the Changes
Sequence Diagram(s)sequenceDiagram
participant Parser
participant Visitor
participant AstNode
Parser->>Visitor: visitUnaryExpr(ctx)
alt ctx has RETURN_TOK
Visitor->>AstNode: create ReturnAst(visit(ctx.expr()))
Visitor->>Parser: return ReturnAst
else other unary
Visitor->>AstNode: create UnaryAst(visit(ctx.expr()))
Visitor->>Parser: return UnaryAst
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/org/piccode/ast/PiccodeVisitor.java (1)
599-612: Good defensive programming with null checks.The null checks improve robustness, though the grammar rules suggest these shouldn't be null under normal parsing. Consider using the key-value pair's start token for more precise error location instead of the parent context's start token.
- var tok = key_val_pairs.getStart(); + var tok = kv.getStart(); throw new PiccodeException(fileName, tok.getLine(), tok.getCharPositionInLine(), "Missing a key in object literal");- var tok = key_val_pairs.getStart(); + var tok = kv.getStart(); throw new PiccodeException(fileName, tok.getLine(), tok.getCharPositionInLine(), "Missing expression in object literal");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/antlr4/PiccodeScript.g4(4 hunks)src/main/java/org/piccode/ast/PiccodeVisitor.java(2 hunks)
🔇 Additional comments (5)
src/main/antlr4/PiccodeScript.g4 (4)
49-49: Good flexibility improvement for default argument values.Allowing any expression as a default value instead of just literals provides more flexibility. The optional
USEprefix adds new functionality for argument handling.
96-96: LGTM! Token rename properly implemented.The change from
RETURNtoRETURN_TOKis properly implemented with both the grammar rule update and the corresponding token definition.Also applies to: 177-177
101-101: Clean syntax improvement for if expressions.Removing the braces simplifies the if expression syntax, making it more consistent with functional programming languages.
54-57: Verify Impact of::and Function-Call PrecedenceAutomated searches did not uncover any instances of the namespace operator immediately followed by a function call (e.g.
a::b(c)) in the repository. Absence of matches may simply mean files use unrecognized extensions or live outside the default search paths.Please manually review any Piccode script examples or test cases for this pattern to ensure the grammar change doesn’t alter intended behavior.
Suggested search commands:
• rg -n '::[[:alnum:]]+\s*(' -g '.pcs' -g '.piccod' -g '*.picode'
• rg -n '::[[:alnum:]]+\s*(' -g 'src/**/*.g4'No changes are required in
src/main/antlr4/PiccodeScript.g4beyond the reviewed reordering, but confirm there are no upstream scripts relying on the old(a::b)(c)parse.src/main/java/org/piccode/ast/PiccodeVisitor.java (1)
527-534: Proper implementation of RETURN_TOK handling.The visitor correctly handles the new
RETURN_TOKtoken by creating aReturnAstnode, which aligns with the grammar changes.
Summary by CodeRabbit
New Features
Bug Fixes