Re: Lookahead correction for the C++ skeleton lalr1.cc

[Adrian Vogelsgesang]

Hi Akim,

> But it does not fit the definition of a "trivial change": to be allowed
> to install these changes, I need you to sign a disclaimer to the FSF, so
> that the FSF is sure that no one will ever start legal action again them
> for using piece of code they are not allowed to use.
>
> Are you ok with sign the disclaimers? I'll send you the forms.

Yes, I am ok with signing the disclaimers. I will keep you updated on
the progress.

> You have a change in yacc.c that I need to look at more carefully.

Yeah, that one is independent of the rest of the commit and I should
probably have put it into a separate commit. It is a refactoring to make
code more readable (at least to me). It should be a refactoring only on
the m4-level, it should not change the generated code at all.

> This is excellent news! I was unaware you were using Bison, and using
> the C++ backend. I'll try to have a look at your grammar (I'm interested
> in benches currently).

Hyper is not open-source, so you won’t be able to find its grammar online.
Structurally our grammar is very similar to Postgres’ grammar
(https://github.com/postgres/postgres), though.

>> The patch does not yet add any new test cases to bison’s test suite since I 
>> wasn’t
>> sure how to do so.
> Yeah, it's kind of tricky. When you know it, it's very nice to avoid
> code duplication in your tests, but the learning curve is very steep.
> I'm try to spend some time on the LAC tests to make them easier to
> run on other backends.

That would be awesome. Thanks for your help!
As you might have noticed from the timestamps, I had those commits
laying around for roughly 6 months now, but didn’t want to submit them
without proper test coverage – after those 6 months, I finally gave up…


Cheers,
Adrian

From: Akim Demaille <address@hidden>
Date: Tuesday, 9 July 2019 at 22:35
To: Adrian Vogelsgesang <address@hidden>
Cc: "address@hidden" <address@hidden>
Subject: Re: Lookahead correction for the C++ skeleton lalr1.cc

Hi Adrian,

> Le 9 juil. 2019 à 14:12, Adrian Vogelsgesang <address@hidden> a écrit :
>
> Dear bison community,
>
> cxx-table-indentation.patch is really minor: it fixes the indentation of the 
> generated
> tables `yypact_` and friends in the headers for the lalr1.cc<http://lalr1.cc> 
> skeleton.

I can apply that guy easily.

> cxx-lac.patch is more interesting:
> This patch adds support for Lookahead Correction to the 
> lalr1.cc<http://lalr1.cc> C++ skeleton.
> It contains pretty much a direct port of LAC from the yacc.c C skeleton.

This is a wonderful piece of news!

But it does not fit the definition of a "trivial change": to be allowed
to install these changes, I need you to sign a disclaimer to the FSF, so
that the FSF is sure that no one will ever start legal action again them
for using piece of code they are not allowed to use.

Are you ok with sign the disclaimers? I'll send you the forms.


> The largest difference from the C skeleton's LAC is probably memory 
> management:
> In C++, I simply reused the existing `stack` class while the C equivalent does
> memory management through `yy_lac_stack_realloc` etc.
> Also, the C++ port uses functions instead of macros for `yy_lac_establish_`,
> `yy_lac_discard_`.

All the better :) It will also make it easier for other people to
cary it over from C++ to say D or Java.


- state_type yy_lr_goto_state_ (state_type yystate, int yysym);
+ static state_type yy_lr_goto_state_ (state_type yystate, int yysym);

Wow... Of course... Thanks!


Your contributions looks very nice! Good job, thanks!

You have a change in yacc.c that I need to look at more carefully.


> All existing bison tests are passing and I am quite sure that the patch works 
> as expected
> because integrating a C++ parser with those patches into Hyper (a SQL 
> database;
> https://www.tableau.com/products/new-features/hyper;<https://www.tableau.com/products/new-features/hyper;>
>  https://hyper-db.de<https://hyper-db.de>) went smoothly
> and our internal test suite for Hyper is still passing. (Only test failures: 
> the list of expected
> tokens is now correct and our test cases were expecting the incorrect list in 
> some places)

This is excellent news! I was unaware you were using Bison, and using
the C++ backend. I'll try to have a look at your grammar (I'm interested
in benches currently).


> The patch does not yet add any new test cases to bison’s test suite since I 
> wasn’t
> sure how to do so.

Yeah, it's kind of tricky. When you know it, it's very nice to avoid
code duplication in your tests, but the learning curve is very steep.
I'm try to spend some time on the LAC tests to make them easier to
run on other backends.

Again, thanks a lot!

Cheers!