[m-dev.] for review: line numbers in the debugger (part 1)

[Fergus Henderson]

On 10-Nov-1999, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Index: doc/user_guide.texi
> + at item break [-PS] @var{filename}:@var{linenumber}
> +Puts a break point on the specified line of the specified file,

I suggest s/file/source file/

> + at sp 1
> + at item context none
> +When reporting events or ancestor levels,
> +does not print contexts (filename/line number pairs).
> + at sp 1
> + at item context before
> +When reporting events or ancestor levels,
> +prints contexts (filename/line number pairs)
> +before the identification of the event or call to which they refer,
> +on the same line.
> +With long fully qualifiied predicate and function names,
> +this may make the line wrap around.

s/ii/i/

> + at sp 1
> + at item context after
> +When reporting events or ancestor levels,
> +prints contexts (filename/line number pairs)
> +after the identification of the event or call to which they refer,
> +on the same line.
> +With long fully qualifiied predicate and function names,
> +this may make the line wrap around.

s/ii/i/

> +++ mercury_stack_trace.c	1999/11/10 03:41:43
>  
> -static	void	MR_dump_stack_record_init(void);
> +static	void	MR_dump_stack_record_init(bool);

It would be a good idea to declare the name of the bool
parameter here.

> Index: runtime/mercury_stack_trace.h
...
> +typedef	void		(*MR_Print_Stack_Record)(FILE *, 
> +				const MR_Stack_Layout_Entry *, 
> +				int, int, Word *, Word *,
> +				const char *, int, bool);
> +

Likewise here it might be a good idea to use names to document the
meaning of the parameters.

> +		} else if (word_count == 2 &&
> +				MR_parse_source_locn(words[1], &file, &line))
> +		{
> +			int	slot;
> +
> +			slot = MR_add_line_spy_point(action, file, line);
> +			if (slot >= 0) {
> +				MR_print_spy_point(slot);
> +			} else {
> +				fflush(MR_mdb_out);
> +				fprintf(MR_mdb_err,
> +					"There is no event at %s:%d.\n",
> +					file, line);
> +			}
> +		} else if (word_count == 2 &&
> +				MR_trace_is_number(words[1], &breakline))
> +		{
> +			int	slot;
> +
> +			if (MR_find_context(layout, &file, &line)) {
> +				slot = MR_add_line_spy_point(action, file,
> +					breakline);
> +				if (slot >= 0) {
> +					MR_print_spy_point(slot);
>  				} else {
> +					fflush(MR_mdb_out);
> +					fprintf(MR_mdb_err,
> +						"There is no event or call "
> +						"at %s:%d.\n",
> +						file, line);
> +				}
> +			} else {
> +				fatal_error("cannot find current filename");
> +			}
> +		} else {
>  			MR_trace_usage("breakpoint", "break");
>  		}

The error messages here are inconsistent: "event" vs "event or call".

I think "event" is probably sufficient, since calls are after all
a kind of event.

> +static bool
> +MR_parse_source_locn(char *word, const char **file, int *line)
> +{
> +	char		*s;
> +	const char	*t;
> +
> +	if ((s = strchr(word, ':')) != NULL) {
> +		for (t = s+1; *t != '\0'; t++) {
> +			if (! MR_isdigit(*t)) {
> +				return FALSE;
> +			}
> +		}
> +
> +		*s = '\0';
> +		*file = word;
> +		*line = atoi(s+1);
> +		return TRUE;
> +	}
> +
> +	return FALSE;
> +}

You should use strrchr() rather than strchr() there.
Otherwise it won't do the right thing with source file names
that contain `:'.

Apart from that, that change looks good.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------