[m-dev.] for review: sorting variables better in the debugger

[Erwan Jahier]

| 
| This change addresses recent comments by our ARGo partners at Dassault.
| It is for review by Erwan; i'd like him to check whether the external
| debugger also works after this change 

I will do.

| (I can't easily test that myself).

Is there something I can do to make that easier in the future?

| 
| Estimated hours taken: 12
| 
| Get the debugger to sort variables better.
| 
| trace/mercury_trace_vars.[ch]:
| 	A new module to manage the debugger's information about the variables
| 	live at the current program point (which can be defined as the
| 	combination of an specific event and an ancestor level).

s/an/a/

| 	This new module centralizes the management of the (sorted) list of
| 	current variables. This in turn allows us to use a better sorting
| 	method, one that orders VarName15 correctly wrt both VarName2 and
| 	VarName.
| 
| 	The new module also uses the type, not the name, of a variable
| 	to decide whether to exclude it from the list of variables
| 	that may be presented to the user (e.g. if it is a typeinfo
| 	or a typeclassinfo).
| 
| trace/mercury_trace_internal.c:
| trace/mercury_trace_external.c:
| 	Use the new module's facilities. This factors out some "mostly"
| 	common code.
| 
| trace/Mmakefile:
| 	Add the new module.
| 
| runtime/mercury_layout_util.[ch]:
| 	Remove the code recently moved here from trace/mercury_trace_internal,
| 	as well as the filtered versions of some functions, since they are
| 	not needed anymore.
| 
| 	Move the code for MR_make_var_list to trace/mercury_trace_external,
| 	since that is the only place where it is used (or is at all likely
| 	to be used). Since its new home is next to several very similar
| 	functions, this should also reduce the probability that any required
| 	maintenance will be overlooked. Also alter its coding style to conform
| 	to its new neighbours.
| 
| 	Move the definition of MR_Var_Spec to trace/mercury_trace_vars.h.
| 
| tests/debugger/*.exp:
| 	Update the expected output files. In most cases, this is just because
| 	the new module outputs different white space, but in some cases, it
| 	also reflects that the debugger now prints variables in a different
| 	order.
| 
| Zoltan.

| Index: trace/mercury_trace_vars.c
| ===================================================================
| RCS file: mercury_trace_vars.c
| diff -N mercury_trace_vars.c
| --- /dev/null	Wed May 28 10:49:58 1997
| +++ mercury_trace_vars.c	Tue May 25 19:46:56 1999
| @@ -0,0 +1,585 @@
| +/*
| +** Copyright (C) 1999 The University of Melbourne.
| +** This file may only be copied under the terms of the GNU Library General
| +** Public License - see the file COPYING.LIB in the Mercury distribution.
| +*/
| +
| +/*
| +** This file contains the code for managing information about the
| +** variables of the program being debugged for both the internal
| +** and external debuggers.
| +**
| +** Main author: Zoltan Somogyi.
| +*/
| +
| +#include "mercury_imp.h"
| +#include "mercury_array_macros.h"
| +#include "mercury_layout_util.h"
| +#include "mercury_trace_vars.h"
| +
| +#include <stdio.h>
| +#include <stdlib.h>
| +#include <string.h>
| +#include <ctype.h>
| +
| +typedef struct {
| +	char				*MR_var_fullname;
| +	char				*MR_var_basename;
| +	char				*MR_var_basename_malloc;
| +	int				MR_var_num_suffix;
| +	bool				MR_var_has_suffix;
| +	bool				MR_var_is_headvar;
| +	bool				MR_var_is_ambiguous;
| +	int				MR_var_hlds_number;
| +	Word				MR_var_value;
| +	Word				MR_var_type;
| +} MR_Var_Details;
| +
| +typedef struct {
| +	const MR_Stack_Layout_Label	*MR_point_top_layout;
| +	Word				*MR_point_top_saved_regs;
| +	const char			*MR_point_problem;
| +	int				MR_point_level;
| +	const MR_Stack_Layout_Entry	*MR_point_level_entry;
| +	Word				*MR_point_level_base_sp;
| +	Word				*MR_point_level_base_curfr;
| +	int				MR_point_var_count;
| +	int				MR_point_var_max;
| +	MR_Var_Details			*MR_point_vars;
| +} MR_Point;
| +
| +static	bool	MR_trace_type_is_ignored(Word type_info);
| +static	int	MR_trace_compare_var_details(const void *arg1,
| +			const void *arg2);
| +static	void	MR_trace_browse_var(FILE *out, MR_Var_Details *var,
| +			MR_Browser browser);
| +static	int	MR_trace_print_var_name(FILE *out, MR_Var_Details *var);
| +
| +#define	MR_INIT_VAR_DETAIL_COUNT	20
| +#define	MR_TRACE_PADDED_VAR_NAME_LENGTH	23
| +
| +static	MR_Point			MR_point;
| +
| +/*
| +** These extern declarations are necessary because the modules defining
| +** these structures (some which are in Mercury and some of which are in C)
| +** do not export them. The types are a lie, but a safe lie.
| +*/
| +
| +extern	Word	mercury_data_private_builtin__type_ctor_info_type_info_1;
| +extern	Word	mercury_data_private_builtin__type_ctor_info_type_ctor_info_1;
| +extern	Word	mercury_data_private_builtin__type_ctor_info_typeclass_info_1;
| +extern	Word	mercury_data_private_builtin__type_ctor_info_base_typeclass_info_1;

Fergus would say to keep lines < 80 columns ;)

| +extern	Word	mercury_data___type_ctor_info_func_0;
| +extern	Word	mercury_data___type_ctor_info_pred_0;
| +extern	Word	mercury_data___type_ctor_info_void_0;
| +extern	Word	mercury_data___type_ctor_info_succip_0;
| +extern	Word	mercury_data___type_ctor_info_hp_0;
| +extern	Word	mercury_data___type_ctor_info_curfr_0;
| +extern	Word	mercury_data___type_ctor_info_maxfr_0;
| +extern	Word	mercury_data___type_ctor_info_redoip_0;
| +extern	Word	mercury_data___type_ctor_info_redofr_0;
| +
| +static	Word *
| +MR_trace_ignored_type_ctors[] =
| +{
| +	/* we ignore these until the debugger can handle their varying arity */
| +	(Word *) &mercury_data_private_builtin__type_ctor_info_type_info_1,
| +	(Word *) &mercury_data_private_builtin__type_ctor_info_type_ctor_info_1,
| +	(Word *) &mercury_data_private_builtin__type_ctor_info_typeclass_info_1,
| +	(Word *) &mercury_data_private_builtin__type_ctor_info_base_typeclass_info_1,

Idem.

| +
| +	/* we ignore these until the debugger can print higher-order terms */
| +	(Word *) &mercury_data___type_ctor_info_func_0,
| +	(Word *) &mercury_data___type_ctor_info_pred_0,
| +
| +	/* we ignore these because they should never be needed */
| +	(Word *) &mercury_data___type_ctor_info_void_0,
| +
| +	/* we ignore these because they are not interesting */
| +	(Word *) &mercury_data___type_ctor_info_succip_0,
| +	(Word *) &mercury_data___type_ctor_info_hp_0,
| +	(Word *) &mercury_data___type_ctor_info_curfr_0,
| +	(Word *) &mercury_data___type_ctor_info_maxfr_0,
| +	(Word *) &mercury_data___type_ctor_info_redoip_0,
| +	(Word *) &mercury_data___type_ctor_info_redofr_0,
| +};
| +
|




| +const char *
| +MR_trace_browse_all(FILE *out, FILE *err, MR_Browser browser)
| +{
| +	int				i;
| +
| +	if (MR_point.MR_point_problem != NULL) {
| +		return MR_point.MR_point_problem;
| +	}
| +
| +	if (MR_point.MR_point_var_count == 0 && out != NULL) {
| +		fprintf(out, "mdb: there are no live variables.\n");
| +	}
| +
| +	for (i = 0; i < MR_point.MR_point_var_count; i++) {
| +		MR_trace_browse_var(out, &MR_point.MR_point_vars[i], browser);
| +	}
| +
| +	return NULL;
| +}
| +
| +static void
| +MR_trace_browse_var(FILE *out, MR_Var_Details *var, MR_Browser browser)
| +{
| +	int	len;
| +
| +	if (out != NULL) {
| +		/*
| +		** The initial blanks are to visually separate
| +		** the variable names from the prompt.
| +		*/
| +
| +		fprintf(out, "%7s", "");
| +		len = MR_trace_print_var_name(out, var);
| +		while (len < MR_TRACE_PADDED_VAR_NAME_LENGTH) {
| +			fputc(' ', out);
| +			len++;
| +		}

MR_trace_browse_var() is called from MR_trace_browse_one() which is called from
MR_trace_browse_one_external(). The old MR_trace_browse_one_external() was only
sending terms on the socket; I can't convince myself that a term will be sent
here. Will it?

More generally, did you make sure all "the mostly common code" you factored out
respect the fact that the external debugger process only reads terms on the
socket?


| Index: trace/mercury_trace_vars.h
| ===================================================================
| RCS file: mercury_trace_vars.h
| diff -N mercury_trace_vars.h
| --- /dev/null	Wed May 28 10:49:58 1997
| +++ mercury_trace_vars.h	Tue May 25 19:47:08 1999
| @@ -0,0 +1,105 @@
| +/*
| +** Copyright (C) 1999 The University of Melbourne.
| +** This file may only be copied under the terms of the GNU Library General
| +** Public License - see the file COPYING.LIB in the Mercury distribution.
| +**
| +** This module looks after the debugger's information about the variables
| +** that are live at a given program point.
| +**
| +** When execution arrives at an event, the debugger should call the function
| +** MR_trace_point_init_vars to initialize this module's data structures
| +** to reflect the variables that are live at that event. During the processing
| +** of the various debugger commands while at that event, the debugger may
| +** call MR_trace_set_level zero or more times to change this module's notion
| +** of the "current" set of variables to refer instead to the variables that
| +** are live at the return address in a given ancestor. This module maintains
| +** its own record of what the current ancestor level is; the enquery function

enquery is not in my dictionnary. Maybe you mean enquiry?

| +** MR_trace_current_level returns this information, while enquery function

Idem.

| +** MR_trace_current_level_details returns information about this level.
| +**
| +** The five functions MR_trace_var_count, MR_trace_list_vars,
| +** MR_trace_return_var_info, MR_trace_browse_one and MR_trace_browse_all
| +** all work in the context established by the MR_trace_init_point_vars and
| +** possibly MR_trace_set_level.
| +**
| +** This context may say that there is no information available about
| +** the variables live at the current location (this is possible if the
| +** relevant module was not compiled with the right debugging flags).
| +** If this is the case, or if some other reason prevents these functions
| +** from carrying out their assigned tasks, most of these functions return
| +** a non-NULL string describing the problem; they return NULL if everything
| +** went OK. (MR_trace_set_level also returns a pointer to an error message
| +** -and refuses to change levels- if something goes wrong.)
| +*/
| +
| +#ifndef	MERCURY_TRACE_VARS_H
| +#define	MERCURY_TRACE_VARS_H
| +
| +#include <stdio.h>
| +#include "mercury_types.h"
| +#include "mercury_stack_layout.h"
| +
| +typedef	void	(*MR_Browser)(Word type_info, Word value);
| +
| +typedef	enum {
| +	MR_VAR_SPEC_NUMBER,
| +	MR_VAR_SPEC_NAME
| +} MR_Var_Spec_Kind;
| +
| +typedef struct {
| +	MR_Var_Spec_Kind	MR_var_spec_kind;
| +	int			MR_var_spec_number; /* valid if VAR_NUMBER */
| +	const char		*MR_var_spec_name;  /* valid if VAR_NAME   */

s/VAR_NUMBER/MR_VAR_SPEC_NUMBER/
s/VAR_NAME/MR_VAR_SPEC_NAME/


| +} MR_Var_Spec;
| +
| +extern	void		MR_trace_init_point_vars(const MR_Stack_Layout_Label
| +				*top_layout, Word *saved_regs);
| +extern	const char	*MR_trace_set_level(int ancestor_level);
| +extern	int		MR_trace_current_level(void);
| +extern	void		MR_trace_current_level_details(
| +				const MR_Stack_Layout_Entry **entry_ptr,
| +				Word **base_sp_ptr, Word **base_curfr_ptr);
| +
| +/*
| +** Return the number of live variables at the current point. If the required
| +** information is missing, return a negative number.
| +*/
| +
| +extern	int		MR_trace_var_count(void);
| +
| +/*
| +** Print the list of the names of variables live at the current point
| +** on the given file.
| +*/
| +
| +extern	const char	*MR_trace_list_vars(FILE *out);
| +
| +/*
| +** Return the name, type and value of the specified variable in the specified
| +** locations, except those which are NULL.
| +*/
| +
| +extern	const char	*MR_trace_return_var_info(int n, const char **name_ptr,

This line exceeds 80 columns.

| +				Word *type_info_ptr, Word *value_ptr);
| +
| +/*
| +** Print the (names and) values of the specified variables.
| +** The names are printed to the given file if the file pointer is non-NULL.
| +** The values are printed by giving them to the specified browser.
| +** The last argument governs whether this function returns an error
| +** if the given variable specification is ambiguous.
| +*/
| +
| +extern	const char	*MR_trace_browse_one(FILE *out, MR_Var_Spec var_spec,

This line exceeds 80 columns.

| +				MR_Browser browser, bool must_be_unique);
| +
| +/*
| +** Print the list of the names and values of all variables live at the current
| +** point. The variables names are printed directly to the given file; the
| +** variable values are printed by calling the given browser function on them.
| +*/
| +
| +extern	const char 	*MR_trace_browse_all(FILE *out, FILE *err,
| +				MR_Browser browser);
| +
| +#endif	/* MERCURY_TRACE_VARS_H */
| cvs diff: Diffing trial
| cvs diff: Diffing util


I am happy with that change as long as it doesn't break the external debugger
(which i am not absolutely convinced though).

Maybe the simplest thing to do is to let you commit that change and then I will
fix problems with the external debugger, if any exists.

--------------------------------------------------------------------------
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
--------------------------------------------------------------------------