Introduce new syntax - #4
Conversation
…ease, debug, unknown)
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new syntax specification for timed regular expressions based on tree-sitter parsing. The changes add comprehensive support for parsing the new Symon specification language while maintaining backward compatibility with the existing dot file format.
Key changes include:
- Implementation of a new tree-sitter-based parser (
SymonParser) for the experimental Symon syntax - Addition of extensive test coverage for the new parser functionality including string constraints, timing constraints, and various automaton operations
- Enhanced type safety and const-correctness improvements across the codebase
Reviewed Changes
Copilot reviewed 58 out of 60 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/symon_parser.hh | Core implementation of the new tree-sitter-based parser with comprehensive parsing logic |
| test/symon_parser_test.cc | Extensive test suite covering all aspects of the new parser functionality |
| src/main.cc | Integration of new parser with command-line interface via --new flag |
| src/automata_operation.hh | New file implementing automaton operations (conjunction, disjunction, concatenation, etc.) |
| test/timing_constraint_test.cc | Unit tests for timing constraint functionality |
| test/parametric_timing_constraint_test.cc | Tests for parametric timing constraints |
| Multiple fixture files | Test fixtures for various automaton types to support comprehensive testing |
Comments suppressed due to low confidence (1)
test/symon_parser_test.cc:1
- Debug output statements should be removed from production code or wrapped in conditional compilation directives to avoid cluttering stderr in normal operation.
#ifndef mem_fun_ref
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| TSParser *inParser = ts_parser_new(); | ||
| ts_parser_set_language(inParser, tree_sitter_symon()); | ||
| const TSTree *tree = ts_parser_parse_string(inParser, nullptr, content.c_str(), content.length()); | ||
| ts_parser_delete(inParser); |
There was a problem hiding this comment.
The indentation uses a tab character instead of spaces. This is inconsistent with the rest of the codebase which uses spaces for indentation.
| ts_parser_delete(inParser); | |
| ts_parser_delete(inParser); |
| std::string skipped; | ||
| std::getline(is, skipped); | ||
| std::cerr << "Undefined action: " << action << std::endl; | ||
| std::cerr << "Undefined action: " << action.c_str() << std::endl; |
There was a problem hiding this comment.
The .c_str() call is unnecessary here since std::string can be directly streamed to std::cerr. This simplifies the code: std::cerr << \"Undefined action: \" << action << std::endl;
| std::cerr << "Undefined action: " << action.c_str() << std::endl; | |
| std::cerr << "Undefined action: " << action << std::endl; |
| const auto frac = p % offset; | ||
| const auto absFrac = abs(frac); |
There was a problem hiding this comment.
[nitpick] The variable frac is only used to compute absFrac. Consider combining these operations: const auto absFrac = abs(p % offset); to reduce intermediate variables.
| const auto frac = p % offset; | |
| const auto absFrac = abs(frac); | |
| const auto absFrac = abs(p % offset); |
This pull request introduces a new specification syntax based on timed regular expressions. The grammar definition is available in https://github.com/maswag/tree-sitter-symon