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

Allow white space in yymore and yyreject checks #608

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

taniwha
Copy link
Contributor

@taniwha taniwha commented Nov 16, 2023

AC_PROG_LEX puts a space between yymore and its ()s causing the yymore check to fail and thus break configure.

AC_PROG_LEX puts a space between yymore and its ()s causing the yymore
check to fail and thus break configure.
@westes
Copy link
Owner

westes commented Dec 8, 2023

I'm confused on where/when configure is broken. I just rebuilt configure and ran it from my local copy of the git repo. It completed successfully.

@d125q
Copy link

d125q commented Dec 10, 2023

It is broken in the sense that AC_PROG_LEX does not detect flex built from HEAD (8453b08 at the time of writing).

AC_PROG_LEX generates the following program:

%{
#ifdef __cplusplus
extern "C"
#endif
int yywrap(void);
%}
%%
a { ECHO; }
b { REJECT; }
c { yymore (); }
d { yyless (1); }
e { /* IRIX 6.5 flex 2.5.4 underquotes its yyless argument.  */
#ifdef __cplusplus
    yyless ((yyinput () != 0));
#else
    yyless ((input () != 0));
#endif
  }
f { unput (yytext[0]); }
. { BEGIN INITIAL; }
%%
#ifdef YYTEXT_POINTER
extern char *yytext;
#endif
int
yywrap (void)
{
  return 1;
}
int
main (void)
{
  return ! yylex ();
}

As you can see, the parentheses are preceded by a space. This causes flex to fail with:

conftest.l:10:3: error: use of undeclared identifier 'yymore_used_but_not_detected'
{ yymore (); }
  ^
conftest.c:614:18: note: expanded from macro 'yymore'
#define yymore() yymore_used_but_not_detected
                 ^
lex.yy.c:1704:25: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
                if ( (yy_buffer_stack == NULL) ) {
                      ~~~~~~~~~~~~~~~~^~~~~~~
lex.yy.c:1704:25: note: remove extraneous parentheses around the comparison to silence this warning
                if ( (yy_buffer_stack == NULL) ) {
                     ~                ^      ~
lex.yy.c:1704:25: note: use '=' to turn this equality comparison into an assignment
                if ( (yy_buffer_stack == NULL) ) {
                                      ^~
                                      =
1 warning and 1 error generated.
configure:14088: $? = 1

For the record, I was attempting to build bedeb7dc of wget.

@westes
Copy link
Owner

westes commented Dec 10, 2023

What are the exact steps I need to follow to reproduce the problem?

@d125q
Copy link

d125q commented Dec 10, 2023

Create a configure.ac file:

AC_INIT
AC_PROG_LEX([noyywrap])
AS_IF([test "x$LEX" != xflex], [AC_MSG_ERROR([flex not found])])
AC_OUTPUT

Run:

$ autoreconf -ifv
$ ./configure

Result:

  • Works with flex tag v2.6.4 as well as the patch in this PR.
  • Fails with HEAD (8453b08 at the time of writing).

@Mightyjo
Copy link
Contributor

Prior to 2.6.4, we didn't include the parentheses in the yymore check at all. I think returning to that construction is the best solution.

This patch will also work, but it prefers C-like function calls. Since I'm slowly working on other output language backends I'm biased toward solutions that will work more generally.

Thanks for pointing this out!

@westes
Copy link
Owner

westes commented Apr 6, 2024

Researching, commit 191d706 is where the particular change was made for yymore.

@Mightyjo
Copy link
Contributor

Mightyjo commented Apr 7, 2024

Sitting with this for a few months, I'm happy with @d125q's solution. I was concerned that using {OPTWS} in the rules would lead to line length problems like those in #620, but we already use this construction in scan.l without trouble so I'm withdraw that concern.

If we ever support an action language that doesn't put parentheses around its argument/parameter lists, I expect we'll be able to search for this thread.

@westes
Copy link
Owner

westes commented Apr 8, 2024

I'm working up a test case for this. It's subtler to sort out that I'd like, so I want to be sure we're able to catch it from happening again.

@westes westes merged commit d5770a0 into westes:master Apr 23, 2024
3 checks passed
@westes
Copy link
Owner

westes commented Apr 23, 2024

Thanks for this; it's now on master.

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.

4 participants