Migration/runtime package#13
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the compiler architecture by introducing a new HLIR (High-Level Intermediate Representation) system with functional error handling using Result types. The old slang.slast package is replaced with slang.hlir, the interpreter is moved to a dedicated slang.runtime package, and a Transform-based pipeline is introduced for composable compiler phases.
Key Changes:
- Introduced Result/Transform types for functional error handling and composable compiler pipelines
- Migrated AST definitions from slang.slast to slang.hlir with renamed types (CompilationUnit → ProgramUnit)
- Extracted interpreter logic into slang.runtime package with separate Value, State, and ControlFlow types
- Renamed packages: slang.visualizer → slang.ui, Error → CompilerError
Reviewed Changes
Copilot reviewed 30 out of 54 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/util/Util.kt | New utility defining Result type and Transform interface for functional composition |
| src/main/kotlin/slang/runtime/* | New runtime package with Interpreter, Value types, State, and ControlFlow |
| src/main/kotlin/slang/parser/Parser.kt | New parser classes with Transform interface implementation |
| src/main/kotlin/slang/hlir/Ast.kt | Renamed AST definitions (CompilationUnit → ProgramUnit, moved from slang.slast) |
| src/main/kotlin/slang/hlir/AstBuilder.kt | Updated AST builder with new type names and helper functions |
| src/main/kotlin/slang/common/CodeInfo.kt | New source location tracking (replaces LineColumn/SourceCodeInfo) |
| src/test/kotlin/*Tests.kt | Updated tests to use new HLIR API and Result types |
| src/main/kotlin/slang/ui/*.kt | Package rename from slang.visualizer to slang.ui |
| src/main/kotlin/slang/repl/Repl.kt | Simplified REPL using new Transform pipeline |
| src/main/kotlin/Main.kt | Updated to use new parser API (renamed class SlangcCLI → SlangCLI) |
Files not reviewed (16)
- .idea/.gitignore: Language not supported
- .idea/.name: Language not supported
- .idea/ANTLRv4ToolGrammarProperties.xml: Language not supported
- .idea/codeStyles/Project.xml: Language not supported
- .idea/codeStyles/codeStyleConfig.xml: Language not supported
- .idea/copilot.data.migration.agent.xml: Language not supported
- .idea/copilot.data.migration.ask.xml: Language not supported
- .idea/copilot.data.migration.ask2agent.xml: Language not supported
- .idea/copilot.data.migration.edit.xml: Language not supported
- .idea/google-java-format.xml: Language not supported
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/misc.xml: Language not supported
- .idea/sonarlint.xml: Language not supported
- .idea/uiDesigner.xml: Language not supported
- .idea/vcs.xml: Language not supported
Comments suppressed due to low confidence (2)
src/main/kotlin/slang/hlir/AstBuilder.kt:351
- Misspelled class name 'ParseTree2HlirTrasnformer' should be 'ParseTree2HlirTransformer' (missing 'n' in 'Transformer').
src/main/kotlin/slang/ui/AstVisualizer.kt:30 - Missing space before function call 'string2hlir' - should be 'val ast = string2hlir(input)'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Compose two transforms: first apply `this`, if it yields Ok then apply [next], otherwise propagate Err. | ||
| */ | ||
| fun <A, B, C> Transform<A, B,>.andThen(next: Transform<B, C>): Transform<A, C> = Transform { input -> |
There was a problem hiding this comment.
Extra comma after type parameter 'B' should be removed.
| is ControlFlow.Normal -> when (flow) { | ||
| else -> flow.value | ||
| } |
There was a problem hiding this comment.
Redundant when expression with only 'else' branch. Should directly access 'flow.value' without the when block.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| @@ -0,0 +1,5 @@ | |||
| class Test { | |||
There was a problem hiding this comment.
Unused class: Test is not referenced within this codebase. If not used as an external API it should be removed.
- Introduce with Value, State, ControlFlow, RuntimeUtils, Interpreter\n- Replace monolithic REPL/interpreter with thin and compatibility wrapper\n- Update compilation rule discovery to use \n- Fix AST builder/visitor issues and small bugs discovered during migration\n\nThis migration centralizes runtime concepts and simplifies future refactors.
jpksh90
force-pushed
the
migration/runtime-package
branch
from
4b17cfe to
368d5df
Compare
|
|
@copilot Review this PR and suggest what can I improve? |
Refactored the whole code base.