On Tue, Feb 21, 2023 at 11:03:59AM -0500, Chet Ramey wrote: (...) > The shell should exit on EOF. Previous versions relied on undocumented > bison behavior, which resulted in a token that wasn't handled by the > grammar.
Thank you for reviewing this and for the explanation. I initially thought this changed as a result of this change: | parse.y | - yylex: return YYUNDEF as current_token if read_token returns < 0. | Fixes parser reset issue reported by Todd Stein <toddbst...@gmail.com> | in https://savannah.gnu.org/support/index.php?110745 I changed `yylex' to return -1 the way it was, and it doesn't make a difference in this case. By luck, I found that removing this line reverts to the original behavior: $ git diff -- parse.y diff --git a/parse.y b/parse.y index 9178e9a7..87d45ac1 100644 --- a/parse.y +++ b/parse.y @@ -3714,7 +3714,7 @@ parse_matched_pair (int qc, int open, int close, size_t *lenp, int flags) free (ret); parser_error (start_lineno, _("unexpected EOF while looking for matching `%c'"), close); EOF_Reached = 1; /* XXX */ - parser_state |= PST_NOERROR; /* avoid redundant error message */ + /*parser_state |= PST_NOERROR;*/ /* avoid redundant error message */ return (&matched_pair_error); } My understanding of how this works is as follows. The unexpected EOF causes `parse_matched_pair' to return an error, which eventually leads to a call to `yyerror'. In `yyerror', we call `report_syntax_error' _only if_ the NOERROR parser flag is off. Despite its name, `report_syntax_error' does more than that. It'll actually clear `EOF_Reached' when the shell is interactive: bash: syntax error: unexpected end of file 6458 if (interactive && EOF_Reached) 6459 EOF_Reached = 0; 6462 last_command_exit_value = (executing_builtin && parse_and_execute_level) ? EX_BADSYNTAX : EX_BADUSAGE; The parser eventually makes it way back to `reader_loop'. In 5.2, EOF_Reached is 1 at that point. In 5.1, it's 0 as it was cleared by `report_syntax_error'. Knowing this now, I was able to find the commit that changed this, and the note explaining the change: | commit b48c234286ae692c758306dac26a5bb72f782fef | Date: Mon Oct 31 10:08:36 2022 -0400 | (...) | parse.y | - parse_matched_pair: set PST_NOERROR if we read to EOF without finding | a closing match and call parser_error; avoids redundant error | message As an aside, this change also restores the original behavior: $ git diff -- parse.y diff --git a/parse.y b/parse.y index 9178e9a7..d2d20874 100644 --- a/parse.y +++ b/parse.y @@ -3713,7 +3713,7 @@ parse_matched_pair (int qc, int open, int close, size_t *lenp, int flags) { free (ret); parser_error (start_lineno, _("unexpected EOF while looking for matching `%c'"), close); - EOF_Reached = 1; /* XXX */ + /*EOF_Reached = 1;*/ /* XXX */ parser_state |= PST_NOERROR; /* avoid redundant error message */ return (&matched_pair_error); } In any case, I do understand why you've made this change. I've checked how bash deals with an EOF while parsing other partial commands (e.g. `if', `for x in', `echo $(') and these all behave in the same way. So I don't disagree. I just thought I should share my troubleshooting notes. > If there's a bug here, it's that interactive shells need to handle > IGNOREEOF in this case. Thanks for reminding me about IGNOREEOF. It does seem like `parse_matched_pair' doesn't work with `ignoreeof' in this case. I suppose that's because the parser doesn't see `yacc_EOF', so it never calls `handle_eof_input_unit'.
