[m-rev.] for review: conditional breakpoints

Zoltan Somogyi zs at cs.mu.OZ.AU
Mon Jan 31 18:40:12 AEDT 2005


On 31-Jan-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> > +	( TermFunctor = CTermFunctor ->
> > +		match_with_cterms(TermArgs, CTermArgs, Match)
> > +	; CTermFunctor = "_" ->
> 
> Would it not be quicker to do this test first?

I don't expect speed to be a problem; the test is executed only
at breakpoints that currently stop. Even if I did, I don't know
which way would be faster; that depends on statistics of 
program characteristics I don't have.

> It'd be nice (and consistent with other parts of mdb) if the argument number
> instead of the var name could be used, though this can be done later.

My plan exactly. That is an independent change.

> Also wouldn't it be better to use "\=" instead of "!=" since "\=" is closer to
> Mercury syntax?

I'll allow both.

> > +    /*
> > +    ** We don't have to worry about skipping white space, because there
> > +    ** won't be any, except inside strings.
> > +    */
> 
> This seems to conflict with the documentation that says a term may contain
> spaces.  Are the spaces removed before the string is passed to this function?

There may be spaces on the command line. The process of breaking the command
line into words deletes them. The only spaces left will be those inside
quotation marks.

> This doesn't seem to handle strings like """" or "\"", or escaped characters
> like `\n'.  Is this handled elsewhere?  If it's not then this limitation
> should be documented in the user guide.

The code for having \ escape the following character was already present
in the command line parser. This means \n is handled OK. At the moment,
you can't usefully escape double quote chars. If you put a \ in front
of them, they are escaped by the command line parser, but the parser
also eats the \, so the code here has no clue the character was escaped.
I documented the limitation.

> Are cases like "f(1 , 2)" handled (where there is a space before
> and after the comma)?

The command line parser deletes all spaces except those inside quotes.

> You should test where the term has more than one argument and where one of the
> arguments is itself a term with arguments.
> 
> Also you should test where the term path or variable doesn't exist and
> the "-v" or "-p" options aren't supplied.

OK. The reason why didn't do them is that the obvious deep structures are 
list and the trivial parser in mercury_trace_term.c can't handle [H | T]
notation.

> There should be a test case for unmatched double quotes.

You can only test for one unmatched quote per test case, because
the only way to terminate an unmatched quote is with EOF. The existing
cmd_quote test case works on single quotes, but the command parser's handling
of unmatched double quotes is exactly symmetric, so I don't think another
test case is worth it. Running mdb by hand yields the expected error message.

I have followed your other suggestions.

Zoltan.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list