Re: remote_expect gets confused by comments, unlike raw expect

[Jacob Bachmeyer]

Pedro Alves wrote:
> This is more of a FYI / brain dump than a request for action.
>
> Another remote_expect issue the gdb testsuite hang exposed (see my
> previous message to this list, and [1]), is that remote_expect
> does not ignore comments like expect does.
>
> If you look at the dejagnu code that implements this underlined
> functionality:
>
>       # remote_expect works basically the same as standard expect, but it
>       # also takes care of getting the file descriptor from the specified
>       # host and also calling the timeout/eof/default section if there is an
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>       # error on the expect call.
>         ^^^^^^^^^^^^^^^^^^^^^^^^
>       #
>       proc remote_expect { board timeout args } {
>
> you'll notice that it will pick the last "eof" section as error section,
> if there are multiple "eof" sections.  But in GDB, it wasn't picking the
> last "eof" section.
>
> The reason is that remote_expect, while parsing the body to select
> the default block to execute after an error, is affected by
> the comments in the body (since they are also parsed).
>
> Compare this version:
>
>  remote_expect {
>    # patterns below apply to any spawn id specified.
>    eof {
>    }
>  }
>
> With this version:
>
>  remote_expect {
>    # The patterns below apply to any spawn id specified.
>    eof {
>    }
>  }
>
> In the second version, remote_expect notices the eof block, while in
> the first it does not.
>
> Any comment at that same place with an even-numbered of tokens
> also works.
>   

I think that we can call this an insane bug.  :-)

> [...]
>
> The fact that comments with an even number of tokens work is just a
> coincidence of how remote_expect's little state machine is
> implemented, which clearly does not consider comments.
>   

That state machine appears to be mostly useless -- its primary output in 
the "res" variable is never actually used, so all it actually does is 
extract an "error" handler into the "error_sect" variable.  The code 
goes all the way back to "Initial revision" with the only changes since 
then being parts of various global style cleanups.

> However, raw expect _does_ accept comments in that position.
> For example, this works as intended:
>
>   expect {
>       # Some comment here.
>       -re "match me" {
>         puts "got it"
>       }
>   }
>
> so it's also not unreasonable to expect that they work
> correctly with remote_expect too.
>
> I argue that remote_expect should follow raw expect here, and
> thus explicitly (and correctly, unlike today) ignore comments.
>   

Would this pattern match comments?  {***:(?n)^[[:space:]]*#.*$}

The problem comes from processing the arguments as a list:  newline is 
whitespace like any other in Tcl lists and gets collapsed with other 
token-delimiting characters, while comments begin with a "#" token and 
end at newline.  This is easy to parse in C; not so easy in Tcl.

(Tcl syntax is a bit weird here and this is the reason that the comments 
in the test lists in testsuite/launcher.all/command.exp are introduced 
with "#" instead of a bare # -- Tcl considers them equivalent in that 
context, but bare # confuses Emacs!)

> I tried to come up with a quick hack for this on the gdb side, one
> that would strip out comment lines before calling remote_expect, but
> I couldn't get it to work 100% correctly.  More details on the thread
> I pointed at before [1].
>
>
> Today I took a look at expect's sources, to see how they do it, and
> found that they treat the arguments to expect's code block as commands,
> here:
>
> [...]
>
> Since in TCL comments can (only) appear where commands
> appear, Tcl_ParseCommand handles/skips comments too.
>
> So I guess that a proper fix for remote_expect would employ a
> similar solution, though I don't have a good grasp on how easy it
> would be to mimic a minimal version of Tcl_ParseCommand in tcl.
>   

Tcl_ParseCommand is written in C, which has very strong facilities for 
crawling byte-by-byte through strings; facilities that Tcl mostly lacks.

The options that I see right now are either:

1.  Split the input on newline; filter the items that begin with 
{[[:space:]]*#}; reassemble and then parse as list.
2.  Use an RE and regsub(n) to strip out comments; then parse as list, 
but this could strip "comment-like" text from other contexts.
3.  Split the input on newline; use [info complete] to reassemble 
elements token-by-token while discarding lines that begin with 
{[[:space:]]*#}.

Some form of option (3) looks best to me, but is likely to be relatively 
slow.  Could there be some shortcut that I am not seeing, since 
remote_expect is essentially a pass-through to expect itself?  I doubt 
that expect will do anything useful with a pre-parsed list, instead 
simply flattening it to a string and parsing again.

I have added rewriting remote_expect to my TODO list, but I will need a 
good description of what remote_expect is actually supposed to *do* 
before I can do that.

-- Jacob