Skip to content
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

C++ support for custom error messages #28

Merged
merged 5 commits into from Feb 27, 2020

Conversation

vogelsgesang
Copy link
Contributor

See mailing list for discussion

The first 8 commits by @akimd in this review are coming from my rebase on the official upstream repo (https://git.savannah.gnu.org/git/bison.git). I guess the master here on github is not up-to-date with the upstream repo.
In any way: the newly added commits are only the last 4 commits

Copy link
Owner

@akimd akimd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks a lot Adrian!
Could you please address the details I reported?
Also, I'd prefer if you could stick to lowercase for the commit titles.

Cheers!

data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
data/skeletons/lalr1.cc Outdated Show resolved Hide resolved
tests/local.at Show resolved Hide resolved
tests/local.at Show resolved Hide resolved
* data/skeletons/lalr1.cc: here
* etc/bench.pl.in: here
* src/location.c: here
* tests/calc.at: and here
Prefer b4_parse_error_case over the adhoc solution
`m4_case + b4_percent_define_get`. Same for b4_parse_error_bmatch.

* data/skeletons/glr.c: here
* data/skeletons/yacc.c: here
* data/skeletons/lalr1.cc: added support here
* tests/calc.at: added a test case
* data/skeletons/lalr1.cc: added support here
* tests/calc.at: added test cases
* tests/local.at: added yyreport_syntax_error implementation
   for C++ test cases
@vogelsgesang
Copy link
Contributor Author

Your comments should be addressed now with my updated commits

@@ -2267,7 +2267,7 @@ yyreportSyntaxError (yyGLRStack* yystackp]b4_user_formals[)
while ((*yyp = *yyformat))
{
if (*yyp == '%' && yyformat[1] == 's' && yyi < yycount)
{]m4_case(b4_percent_define_get([[parse.error]]), [verbose], [[
{]b4_parse_error_case( [verbose], [[
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, please, remove this space.

, yyla (yyla)
{}
]b4_locations_if([[
const location_type& get_location () const { return yyla.location; }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer location_get (like type_get), or just location.

@akimd
Copy link
Owner

akimd commented Feb 27, 2020

Your comments should be addressed now with my updated commits

You did thanks! Sorry for the minor issues I missed the first time. I will install your changes this evening.

Cheers!

@akimd akimd merged commit 9a7b2d5 into akimd:master Feb 27, 2020
@akimd
Copy link
Owner

akimd commented Feb 27, 2020

I'll fix these final minor issues, and push that onto savannah. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants