feat: Add Antlr to parse the spider task definition language into parse tree. - #168
feat: Add Antlr to parse the spider task definition language into parse tree.#168sitaowang1998 wants to merge 27 commits into
Conversation
WalkthroughThis change introduces ANTLR-based parser generation for a new domain-specific language ("Stdl"), updates build and dependency workflows to support ANTLR and its C++ runtime, and refactors linting and dependency installation tasks for clarity and consistency. It also adds new variables and updates the development utility submodule. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant BuildSystem
participant ANTLR
participant Filesystem
Developer->>BuildSystem: Run build (target)
BuildSystem->>BuildSystem: Run stdl-generate-parser task
BuildSystem->>ANTLR: Invoke ANTLR tool on Stdl.g4
ANTLR->>Filesystem: Generate C++ parser code
BuildSystem->>Filesystem: Clean up and validate generated files
BuildSystem->>BuildSystem: Continue with main build (target)
Possibly related PRs
Suggested reviewers
✨ 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 (
|
|
On hold. Include changes in #166. |
![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
🧹 Nitpick comments (5)
.github/workflows/unit-tests.yaml (1)
53-55: Parallelism hard-coded to 1 – CI will run noticeably slowerFor most dependency builds the runners can safely use several cores. Locking the install step to a single job may add multiple minutes to the workflow. Consider:
- SPIDER_DEP_BUILD_PARALLELISM: "1" +# Pick a conservative default but still allow >1 core + SPIDER_DEP_BUILD_PARALLELISM: "${{ env.SPIDER_DEP_BUILD_PARALLELISM || '2' }}"or expose the value through a matrix parameter so it can be tuned per OS image.
.github/workflows/code-linting-checks.yaml (1)
50-52: Same single-core cap as unit-tests – revisit for lint jobThe lint job is CPU-bound (clang-tidy, clang-format, etc.). Serialising the dependency build stage as well may outweigh the occasional contention problem you are trying to avoid. Re-evaluate the value (
1) once the ANTLR runtime is stable.tools/scripts/lib_install/linux/install-dev.sh (1)
30-30: Use the headless JDK to shave off ~200 MB of packagesUnless you explicitly need Swing/AWT on the runner,
openjdk-11-jdk-headlessis enough for ANTLR and reduces image size + network traffic.- openjdk-11-jdk \ + openjdk-11-jdk-headless \taskfile.yaml (1)
24-25: Defaulting to empty string may propagate as a stray spaceDown-stream commands that expect
-jNstyle flags might receive"-j "(note the space). Provide an explicit default, e.g.:- G_DEP_BUILD_PARALLELISM: >- - {{default "" (env "SPIDER_DEP_BUILD_PARALLELISM")}} + G_DEP_BUILD_PARALLELISM: >- + {{default "1" (env "SPIDER_DEP_BUILD_PARALLELISM")}}so behaviour is deterministic.
src/stdl/parser/Stdl.g4 (1)
13-13: Field syntax is inconsistent with parameter syntax.The field rule has
type IDwhile parameter rule hasID ':' type. This inconsistency might confuse users.Consider making field syntax consistent with parameter syntax:
-field: type ID ; +field: ID ':' type ;This would make struct fields look like
{ name: string, age: int32 }instead of{ string name, int32 age }.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
src/stdl/generated/StdlBaseVisitor.cppis excluded by!**/generated/**src/stdl/generated/StdlBaseVisitor.his excluded by!**/generated/**src/stdl/generated/StdlLexer.cppis excluded by!**/generated/**src/stdl/generated/StdlLexer.his excluded by!**/generated/**src/stdl/generated/StdlParser.cppis excluded by!**/generated/**src/stdl/generated/StdlParser.his excluded by!**/generated/**src/stdl/generated/StdlVisitor.cppis excluded by!**/generated/**src/stdl/generated/StdlVisitor.his excluded by!**/generated/**
📒 Files selected for processing (9)
.github/workflows/code-linting-checks.yaml(1 hunks).github/workflows/unit-tests.yaml(1 hunks)build-tasks.yaml(2 hunks)dep-tasks.yaml(6 hunks)lint-tasks.yaml(3 hunks)src/stdl/parser/Stdl.g4(1 hunks)taskfile.yaml(1 hunks)tools/scripts/lib_install/linux/install-dev.sh(2 hunks)tools/yscope-dev-utils(1 hunks)
🔇 Additional comments (29)
tools/yscope-dev-utils (1)
1-1: Confirm the submodule bump & CI configurationUpdating the submodule pointer is fine, but please double-check:
- The commit
5c6bfbd00ad4e9cbfdfab9708298fb11bda4ae8cis pushed to the upstreamyscope-dev-utilsrepo and not on a private/ephemeral branch.- All CI jobs that rely on submodules run
git submodule update --init --recursive(or equivalent) so the new revision is actually fetched.- Release notes / CHANGELOG mention the submodule bump if it introduces behaviour your consumers must know about (e.g., additional build flags needed for ANTLR).
This prevents broken checkouts when others clone the project.
taskfile.yaml (1)
20-20: DSL source dir variable looks good
G_SRC_DSL_DIRfollows the existing naming scheme and keeps the parser sources well-scoped.build-tasks.yaml (2)
9-9: Dependency ordering 👍Adding
stdl-generate-parserto thetargetdeps ensures generated sources exist before CMake runs – nice catch.
41-48: Undefined variableG_ANTLR_JAR_FILE
java -jar "{{.G_ANTLR_JAR_FILE}}"assumes the global var is available in this context. If a future refactor changes variable scope the task will explode at runtime. Consider:vars: ANTLR_JAR: "{{.G_ANTLR_JAR_FILE}}" # fail early if not set ... java -jar "{{.ANTLR_JAR}}" \and document in the task header which deps export it.
src/stdl/parser/Stdl.g4 (10)
1-3: Grammar structure looks solid.The grammar correctly defines the start rule to handle multiple service and struct declarations followed by EOF.
5-5: Service declaration syntax is clear.The service rule properly defines the expected syntax with keyword, identifier, and function block.
7-7: Function declaration syntax is well-defined.The function rule correctly handles parameter lists and return types with proper arrow syntax.
9-9: Parameter syntax follows standard conventions.The parameter rule properly defines typed parameters with colon separator.
11-11: Consider making trailing comma optional in struct fields.The struct rule allows an optional trailing comma, which is good for flexibility, but the field list requires at least one field.
15-18: Return type rule supports both single and tuple returns.The return_type rule correctly handles both single types and tuple syntax for multiple return values.
20-20: Identifier rule follows standard conventions.The ID lexer rule properly defines identifiers starting with letter/underscore followed by alphanumeric characters.
22-25: Type rule is well-structured.The type rule correctly allows both user-defined types (ID) and builtin types.
27-39: Builtin types are comprehensive.The builtin_type rule covers generic container types (List, Map) and standard primitive types. The type coverage appears complete for a DSL.
41-41: Whitespace handling is correct.The WS rule properly skips whitespace characters including tabs, carriage returns, and newlines.
lint-tasks.yaml (5)
48-48: Good refactoring to separate lint-specific source files.The introduction of
&cpp_lint_source_filesanchor provides better separation of concerns between lint configuration and general source files.
62-65: Explicit ROOT_PATHS definition improves clarity.The new
&cpp_source_filesanchor with explicit directory listing makes the configuration more maintainable and clear.
69-69: Consistent anchor usage across tasks.The use of
*cpp_lint_source_filesand*cpp_source_filesanchors is consistent across all relevant tasks.Also applies to: 76-76, 85-85
93-93: Parameter rename improves consistency.The change from
INCLUDE_PATTERNStoINCLUDE_FILENAME_PATTERNSbetter reflects the actual usage and improves parameter naming consistency.Also applies to: 104-104
95-97: Explicit ROOT_PATHS arrays improve maintainability.The explicit array definition for ROOT_PATHS makes the configuration more readable and maintainable compared to anchor references.
Also applies to: 106-107
dep-tasks.yaml (10)
6-8: ANTLR version and path configuration looks correct.The version 4.13.2 is properly defined and the jar file path follows consistent naming conventions.
23-24: ANTLR tasks properly integrated into dependency chain.The new ANTLR tasks are correctly added to the
install-all-rundependencies.
49-73: ANTLR JAR installation task is well-structured.The task properly handles checksum validation, download, and checksum computation. The structure follows the established pattern for dependency downloads.
75-131: ANTLR runtime installation task is comprehensive.The task properly handles tarball extraction with pattern filtering, CMake configuration, building, and installation. The approach of extracting only the C++ runtime subdirectory is efficient.
39-47: Consistent variable naming improves maintainability.The standardisation of variable names (
CMAKE_PACKAGE_NAME,TAR_SHA256,TAR_URL,CMAKE_GEN_ARGS) across all dependency tasks improves consistency and maintainability.Also applies to: 139-147, 171-180, 191-203, 211-221, 231-242
47-47: Parallel build support enhances performance.The addition of
JOBS: "{{.G_DEP_BUILD_PARALLELISM}}"to all dependency build tasks will improve build performance.Also applies to: 147-147, 180-180, 203-203, 221-221, 242-242
200-200: CMake settings file reference correction.The capitalisation fix from
boost.cmaketoBoost.cmakeensures consistent naming conventions for CMake settings files.Also applies to: 240-240
201-201: CMake policy addition enhances compatibility.The addition of
-DCMAKE_POLICY_DEFAULT_CMP0074=NEWhelps ensure proper CMake behaviour with find_package operations.Also applies to: 241-241
248-248: Boost installation refactoring improves maintainability.The replacement of the large commented block with a single utility task call (
":utils:boost:download-and-install") significantly improves code maintainability.
64-66: Ensure ANTLR JAR checksum accuracyThe ANTLR site does not publish an official SHA256 for the 4.13.2 complete jar, and the value currently in dep-tasks.yaml matches a third-party source (Npackd). To guarantee the integrity of this dependency:
- Download the jar directly from the official URL:
https://www.antlr.org/download/antlr-4.13.2-complete.jar- Compute its SHA256 checksum locally (e.g.
shasum -a 256 antlr-4.13.2-complete.jar)- Update dep-tasks.yaml (lines 64–66) so that
FILE_SHA256reflects your computed value- Confirm the
URLremainshttps://www.antlr.org/download/antlr-{{.G_ANTLR_VERSION}}-complete.jar
|
Change in development plan. |
1 participant
Description
As title.
This PR adds
javainto install scripts andantlr-jarandantlr-runtimeinto dependency install tasks. This PR also adds thestdl(short forSpidertask definition language) grammar and task to generate parser code usingantlr.Checklist
breaking change.
Validation performed
task build:stdl-generate-parser.Summary by CodeRabbit
New Features
Chores