Ticket #760 (new patch)

Opened 5 years ago

Last modified 4 years ago

readline_interactive method no longer returns PMCNULL on eof

Reported by: flh Owned by: whiteknight
Priority: blocker Milestone:
Component: core Version: trunk
Severity: release Keywords:
Cc: Language: tcl
Patch status: rejected Platform: all

Description (last modified by coke) (diff)

Sending EOF to the standard input (typically: hitting Control-G) does not make HLLCompiler end interactive sessions when readline is used.

To reproduce: take your favorite HLL written with HLLCompiler, and a Parrot with readline, and run it without any argument. This should start an interactive session (i.e., you get a prompt for the language), but hitting ^G makes HLLCompiler display a new prompt instead of exiting.

This bug has two origins:

* the "readline_interactive" method of FileHandle always returns a string (that's because of its type: when it seems to return NULL, it is actually converted to the empty string). On the other hand, HLLCompiler tests if the value returned by "readline_interactive" is null to detect EOF: this test is always false... The fix is to test EOF using the "get_bool" vtable of FileHandle; and

* the method "readline_interactive" never sets the EOF flag when readline is activated.

The attached patch fixes this.

Attachments

readline_eof.patch Download (1.1 KB) - added by flh 5 years ago.
Readline sets EOF in FileHandle and HLLCompiler checks EOF

Change History

Changed 5 years ago by flh

Readline sets EOF in FileHandle and HLLCompiler checks EOF

Changed 5 years ago by pmichaud

I vote against this patch, at least as it stands now. The original 1.0.0 behavior of returning NULL upon reaching EOF needs to be restored for the 1.4 release (because it has not been deprecated and HLL's obviously depend on it).

Pm

Changed 5 years ago by pmichaud

  • priority changed from normal to blocker
  • component changed from none to core
  • severity changed from medium to release
  • summary changed from Interactive sessions with HLLCompiler (and readline) never end to readline_interactive method no longer returns PMCNULL on eof

The problem appears to be r39065, which changed readline_interactive to always return a string instead of returning NULL on EOF. This needs to be fixed before 1.4.0.

Pm

Changed 5 years ago by coke

Be very nice if we could get a fix in for the 1.3 release.

Changed 5 years ago by coke

  • lang set to tcl

Changed 5 years ago by NotFound

  • patch changed from new to rejected

After some discussion on irc, reverting to the original behavior of returning PMCNULL on EOF in r39575

Changed 5 years ago by flh

Re-reading myself: it's Control-D of course, not G :)

Returning a PMCNULL does the trick, yet there still is an inconsistency: PIO_F_EOF is not set when readline is used, while it is set upon reaching EOF when readline is not used.

Is there any plan to fix it some day?

Changed 5 years ago by NotFound

Forgot to comment that part. In the irc discussion we decided that we may need a deprecation notice for that changes. We'll discuss it at parrotsketch, create RFC tickets, or both. Stay tuned.

Changed 5 years ago by allison

I don't recall seeing this come up in parrotsketch. Looks like we've restored the 1.0 behavior. IIRC, the readline launched with 1.0 didn't set EOF (and couldn't set it, because there was no way to detect it without reading ahead.)

Changed 5 years ago by NotFound

My fault, forgot to take a note for commenting it at parrotsketch. The behavior has been reverted, yes.

The problem is that the method return a STRING * when not eof and PMCNULL when EOF, so if a string register is used for the returned value we get a PMCNULL access on EOF. Returning NULL makes more sense but breaks existing code, as this ticket shows, so it need a proposal for deprecation.

On the other part, setting the EOF flag *after* reading EOF is a reasonable behavior and can be easily implemented.

Changed 4 years ago by whiteknight

  • owner set to whiteknight

Changed 4 years ago by coke

  • description modified (diff)

fix formatting in desc.

Note: See TracTickets for help on using tickets.