-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Toy v1 parser implementation #162
Merged
rturrado
merged 30 commits into
develop
from
toy-v1-use_case_circuit_for_parser_verification
Oct 2, 2023
Merged
Toy v1 parser implementation #162
rturrado
merged 30 commits into
develop
from
toy-v1-use_case_circuit_for_parser_verification
Oct 2, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e v1x/analyzer, v1x/program, and v1x/ParseResult. - Changed v1x/analyzer API so that it can also be passed a v3x/ParseHelper. - v3x/cqasm free functions use a v1x/analyzer and return a v1x/program, but do the parsing with a v3x/ParseHelper. - v3x/ParseHelper returns a v1x/ParseResult.
The ScannerAntlr::parse_ code merely invokes the lexer and the parser at the moment.
- BuildTreeGenAstVisitor.{h,cpp}: visitor of the AST returned by ANTLR which builds a tree-gen AST (not finished yet!). - cqasm-parse-helper.cpp: added code at ScannerAntlr::parse_ to convert the AST returned by ANTLR into a tree-gen AST. - generate_antlr_parser.py: added a parameter to generate a base visitor class. - Cqasm{Lexer,Parser}.g4: renamed cqasm_lexer.g4 and cqasm_parser.g4 to CqasmLexer.g4 and CqasmParser.ge as these texts are used as class names, and all ANTLR class names are written in camel case. - CqasmParser.g4: renamed expressions from snake case to camel case (for the same reason as above). - test_cases.txt: I will use this as a guide for the integration tests.
Slightly changed the grammar to make look more like the v1x grammar.
- If the input file has 3.0 version, we parse it through the ANTLR parser. Also took the opportunity to refactor test/v1x/parsing.cpp. - Removed dirent.h dependency. Used std::filesystem instead. Updated src/CMakeLists.txt to a tree-gen/v3 version that supports printing of nodes via fmt. - TODO: merge those tree-gen changes into master. Added an empty test/v3x/cqasm-parse-helper.cpp.
Added current branch to GitHub Actions.
std::from_chars is not available until clang 15.
See explanation of fix here: https://stackoverflow.com/a/77120324/260313.
- src/CMakeLists.txt: - Moved find_package calls to the beginning of the Packages section. - Updated to the last version of tree-gen, which allows to print nodes using fmt. - Added target include directories as debug info. - test/v1x/CMakeLists.txt: temporarily commented out adding of tutorial.cpp. - test/CMakeLists.txt: temporarily commented out adding of v10 and v3x subdirectories. - cqasm-version.cpp: simplified operator<< implementation by using fmt. - parsing.cpp: fixed TestBody code. - res/v1x/toy-v1x-parsing/{ast.golden.txt,input.cq}: removed. - test_cases.txt: cleaned up a bit.
…r listener). - src/CMakeLists.txt: - Added 'find_package(fmt 10.1.1)'. - Updated to a version of tree-gen that doesn't fetch fmt if the package is found. - v3x/cqasm-parse-helper: - Changed ScannerAntlr to have a customer error listener as a dependency injection. - Changed the 'parse' API to return a ParseResult. Ideally, a ParseResult should be an interface containing a pointer to a generic AST, not to a tree-gen AST. - v3x/BuildTreeGenAstVisitor: fixed so that all tests are now run. - v3x/CustomErrorListener: added so that the parser throws an exception instead of just printing to standard output. - res/v1x/toy-v1x-parsing: - All implemented tests pass the syntax analysis check. - Updated test_cases.txt. Note: - conanfile.py: temporarily commented out a few lines to be able to work with CLion's Conan Plugin. - test/CMakeLists.txt: temporarily commented out version tests. - test/v1x/parsing.cpp: temporarily commented out semantic checks.
…t_analyzer instead. Updated some tests.
pablolh
suggested changes
Sep 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow!
- BuildCustomAstVisitor.hpp: made methods virtual. - BuildTreeGenAstVisitor.cpp: removed unnecessary checks that should be done by the lexer/parser. - cqasm.cpp: lambdas use references instead of copies now. - cqasm-parse-helper.{hpp,cpp}: moved scanner code into separate files. - ScannerAntlr.{hpp,cpp}: moved scanner code into separate files. - test.yml: fixed comment.
- BuildTreeGenAstVisitor checks for out of range exceptions when converting integer and float string literals. Also, although unrelated, took the opportunity to do some CMake changes: - Added LANGUAGES to 'project(cqasm)' at CMakeLists.txt. - Updated tree-gen to a version that fixes some warnings.
This simplifies a bit the visitor, since you don't need neither the visitStatement nor the visitExpression methods. Furthermore, you avoid having a block consisting of a sequence of if-else statements as implementation for these methods.
Hi Pablo, I think I have addressed most of your comments. We still have to talk about what we want in
|
pablolh
reviewed
Sep 27, 2023
pablolh
previously approved these changes
Sep 27, 2023
…er_literal_value and get_float_literal_value.
pablolh
previously approved these changes
Sep 27, 2023
…he work on the v3 version of the parser is still completely unsupported.
pablolh
previously approved these changes
Sep 27, 2023
…rcuit_for_parser_verification
…cation # Conflicts: # README.md
- v1x/cqasm-analyzer: I've put back the API for parsing and analyzing a file path, a file pointer and a data string. There is at least some code in OpenQL (src/ql/ir/compat/detail/cqasm_reader.cc, ReaderImpl::string2circuit and ReaderImple::file2circuit) that is still using this API, so we shouldn't remove it yet. - test/CMakeLists.txt: commented out v3x tests (they are unimplemented). - test/v1x/parsing.cpp: commented out Toy v1 tests (they would otherwise fail the semantic checks, as they would fail the analyze version check at src/v1x/cqasm-analyzer.cpp; we could temporarily change this check, but I don't think it is worth for this task).
pablolh
previously approved these changes
Sep 29, 2023
pablolh
approved these changes
Oct 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ANTLR 2: visitors, error listeners
This is the second task on the cQASM 3.0 roadmap after the Toy v1 infrastructure (see PR #157).
It has allowed us to focus purely on implementing a minimal ANTLR parser without touching other aspects of the language.
The
cqasm-parse-helper
API:ScannerAntlr
that receives aBuildCustomAstVisitor
and aCustomErrorListener
, and thenParseHelper
that receives aScannerAdaptor
.The
ScannerAntlr
class:The
ParseHelper
class:ScannerAntlr
to parse an input stream, and, whether,Things we don't want in develop
res/v1x/test-v1x-parsing
: I needed something to test against. However all those tests will be lost....cqasm-analyzer.{hpp,cpp}
,cqasm.{hpp,cpp}
,cqasm-py.{hpp,cpp}
: I've removed the analyze functions that received only a file name or a data string from theAnalyzer
. This way, at a higher level, atcqasm.hpp
orcqasm.py.hpp
, I can call theAnalyzer
with a v1 or v3 parser. I.e. we can reuse the v1Analyzer
for the v3 parser. I understand we won't want this functionality in the future.Things we do want in develop
ParseResult
to a separate file.../
from an include header when unnecessary.ParseHelper
, build tree-gen AST visitor, custom error listener...generate_antlr_parser.py
: added a parameter to generate a base visitor class.Cqasm{Lexer,Parser}.g4
: renamedcqasm_lexer.g4
andcqasm_parser.g4
toCqasmLexer.g4
andCqasmParser.g4
as these texts are used as class names, and all ANTLR class names are written in camel case.CqasmParser.g4
: renamed expressions from snake case to camel case (for the same reason as above).test/v1x/parsing.cpp
:dirent.h
dependency. Usedstd::filesystem
instead.fmt::format
instead of standard streams in many places.flie_name
instead offilename
, orfile_path
instead offile_name
when the variable actually holds a path.TODO
parsing.cpp
for v3x. Also, see if they would require some changes in ANTLR.ParseResult
: try changingParseHelper
so that it works with an interface containing a pointer to a generic AST, not to a tree-gen AST.test/v1x/parsing.cpp
: Isversion > cqasm::version::Version("1.2")
working properly?