-
Notifications
You must be signed in to change notification settings - Fork 165
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
[ CPP ] Throw runtime exception on parsing error #288
[ CPP ] Throw runtime exception on parsing error #288
Conversation
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.
Thanks for your work on the CPP backend!
Here is a few comments on how to improve this PR:
- Since the exception concerns only parsing, its definition should not be in the file for the abstract syntax but in the one for the parser.
- Make sure the generated code is properly indented.
- I wonder whether the generated test program also needs to be updated, to catch the exception and print the error message that was used to be printed directly.
- Finally, the desired behavior you are implemented should be secured by a test case. See the
testing
directory for examples of tests for functionality.
5d170fb
to
3c247de
Compare
Hi @andreasabel, I moved the definition of |
3c247de
to
25f2749
Compare
Regarding the tests. To be honest, not that much proficient in Haskell. I would be very pleased if you could help me with tests or at least explain to me how to extend them ;) |
Thanks for the update, @miketsukerman ! At the moment I am a bit snowed under, but I will look at the PR when things are back to normal. |
25f2749
to
afd53d0
Compare
Hi Mike,
One way to run the test is simply to type The generated namespace bnfc
{
class parse_error : public std::runtime_error
{
public:
parse_error(int line, std::string str)
: std::runtime_error(str)
, m_line(line) {}
... using its own namespace But the call to throw foo::parse_error(fooyy_mylinenumber,str); expecting If However, you might have strong reasons to prefer a separate |
afd53d0
to
387af30
Compare
Very simple - just a mistake. Fixed |
Thanks. Er, I meant the rationale of having a separate .H file. |
It was the simplest way to include it in the #include "ParserError.H"
#include "Absyn.H"
#define YYMAXDEPTH 10000000
typedef struct yy_buffer_state *YY_BUFFER_STATE;
int yyparse(void);
int yylex(void);
YY_BUFFER_STATE bnfcyy_scan_string(const char *str);
void bnfcyy_delete_buffer(YY_BUFFER_STATE buf);
int bnfcyy_mylinenumber;
void bnfcinitialize_lexer(FILE * inp);
int bnfcyywrap(void)
{
return 1;
}
void bnfcyyerror(const char *str)
{
extern char *bnfcyytext;
throw bnfc::parse_error(bnfcyy_mylinenumber,str);
} I can't include |
387af30
to
2ab76e2
Compare
Ok, I see, thanks for the explanation! I ran the testsuite on your PR and now there is only a small problem left:
This means that the cleaning goals of the generated I can fix this myself. Thanks for your contribution! |
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.
This is the review I should have done before merging, haha.
The patch introduced some regressions, unfortunately.
- unused variable yytext in parse error function - parse errors should be reported on stderr as before
When BNFC generated code embedded into another project it's more convenient to get parsing errors as C++ exceptions rather than just via standard output.
I've added
parse_error
class to theAbsyn.H