Closed rturrado closed 5 months ago
Many thanks for the review, Pablo.
I may not have gone the whole way with the use of optional
, but it has indeed made the API clearer now. I don't like at all default arguments.
Thanks for this kind of review suggestions.
API methods returning a JSON now follow the Diagnostic structure of the Language Server Protocol (LSP) specification.
We've taken the opportunity to refactor the libqasm's "error handling". Every
ParseResult
andAnalysisResult
manages a vector ofError
objects, instead of a vector of strings. This way, the formatting of an error object is delayed right until the API implementation.All the syntactic error messages start with
Error:
orError at <location>:'
. And many of them include a<last column>
field. This change should fix issue #178.Refactor
error::AnalysisError
:Error
, since this class is used both for parse and analysis errors.ParseError
andAnalysisError
aliases.CustomErrorListener
.to_json
method,operator<<,
and enable the class to be used viafmt
. The JSON string follows the Language Server Protocol (LSP) specification. Every error is mapped to an LSP Diagnostic structure.ostringstream
message member. I find that overcomplicates the class.get_message
method. I find keeping thewhat
method alone is enough.Refactor
annotations::SourceLocation
:fmt
.Refactor
CustomErrorListener::syntaxError
:error::ParseError
instead of astd::runtime_error
with a formatted message. This change should fix issue #178.Refactor
v1x::Analyzer
:analyze(const ParseResult &)
toanalyze(ParseResult &&)
. TODO: I am not very sure about this change. Yes, both v1x and v3x APIs are now consistent. But why remove constness from an API (which is a good thing)?Update references to
error::AnalysisError
in the code.error::ParseError
orerror::AnalysisError
depending on if the code is doing parsing or analysis.Add
.gitattributes
file.Add
cmake-build-*
folders to.gitignore
.