Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

jonas.hahnfeld
Reviewers: ,


https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4
File aclocal.m4 (right):

https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4#newcode556
aclocal.m4:556: # check for yyFlexLexer.yypop_buffer_state () since flex
2.5.29
Random thought: Do we want to check for features introduced in 2003 or
can we just assume that we never come across ancient versions that don't
support this?

https://codereview.appspot.com/563560043/diff/567260043/lily/lexer.ll
File lily/lexer.ll (right):

https://codereview.appspot.com/563560043/diff/567260043/lily/lexer.ll#newcode753
lily/lexer.ll:753: <maininput>{ANY_CHAR} {
David, you added this code in commit 6b0ff05318 ("Make lexer more robust
against unexpected EOF"). Do you have an idea what I have to do to end
up in this code path? I didn't manage to trigger so far :-( I think that
the new code is equivalent to the old one, but would prefer to check on
an example.

Description:
Let flex handle the input stack

This requires at least flex 2.5.29 released in 2003.

Prior cleanups:
1) Use Includable_lexer::new_input in Lily_lexer

Only Includable_lexer::new_input (const string &, Sources *) is virtual,
the method with type (const string &, const string &, Sources *) should
not be declared.

2) Deduplicate code from Includable_lexer::new_input

Please review this at https://codereview.appspot.com/563560043/

Affected files (+41, -87 lines):
  M aclocal.m4
  M config.hh.in
  M lily/includable-lexer.cc
  M lily/include/includable-lexer.hh
  M lily/include/lily-lexer.hh
  M lily/lexer.ll
  M lily/lily-lexer.cc



Reply | Threaded
Open this post in threaded view
|

Re: Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

Han-Wen Nienhuys-3
LGTM




https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4
File aclocal.m4 (right):

https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4#newcode556
aclocal.m4:556: # check for yyFlexLexer.yypop_buffer_state () since flex
2.5.29
On 2020/02/20 14:21:08, hahnjo wrote:
> Random thought: Do we want to check for features introduced in 2003 or
can we
> just assume that we never come across ancient versions that don't
support this?

could we check the version number instead?

https://codereview.appspot.com/563560043/

Reply | Threaded
Open this post in threaded view
|

Re: Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

jonas.hahnfeld
In reply to this post by jonas.hahnfeld

https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4
File aclocal.m4 (right):

https://codereview.appspot.com/563560043/diff/567260043/aclocal.m4#newcode556
aclocal.m4:556: # check for yyFlexLexer.yypop_buffer_state () since flex
2.5.29
On 2020/02/20 23:05:46, hanwenn wrote:
> On 2020/02/20 14:21:08, hahnjo wrote:
> > Random thought: Do we want to check for features introduced in 2003
or can we
> > just assume that we never come across ancient versions that don't
support
> this?
>
> could we check the version number instead?

No, the header doesn't have version information. Moreover I think
checking for functionality is actually better than checking a random
version number because it actually documents what we need.
As I've already adapted the existing test below, I think we can also
leave it for now. The only advantage of deleting them would be to have
less lines ;-)

https://codereview.appspot.com/563560043/

Reply | Threaded
Open this post in threaded view
|

Re: Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

Dev mailing list
In reply to this post by jonas.hahnfeld
> > could we check the version number instead?
>
> No, the header doesn't have version information.

So we need both a recent flex program and a recent flex header, right?
For the former a version test would be easy, see

http://git.savannah.nongnu.org/cgit/autoconf-archive.git/tree/m4/ax_prog_flex_version.m4

For the latter, I think your code completes the tests nicely.


https://codereview.appspot.com/563560043/

Reply | Threaded
Open this post in threaded view
|

Re: Let flex handle the input stack (issue 563560043 by jonas.hahnfeld@gmail.com)

jonas.hahnfeld
In reply to this post by jonas.hahnfeld
On 2020/02/21 08:21:07, lemzwerg wrote:
> > > could we check the version number instead?
> >
> > No, the header doesn't have version information.
>
> So we need both a recent flex program and a recent flex header, right?
 For the
> former a version test would be easy, see
>
>
http://git.savannah.nongnu.org/cgit/autoconf-archive.git/tree/m4/ax_prog_flex_version.m4

Yes, I'm actually working towards a point where we can use the macros
from autoconf-archive ;-)

https://codereview.appspot.com/563560043/