[m-dev.] For review: handling one by one variable retrieval within the external debugger

[Erwan Jahier]

Fergus Henderson wrote:

> These functions should be either declared in a header file
> or defined as `static' and declared near the start of the .c file.

Well in fact, they were declared in mercury_trace_util.h (that's why I
didn't get any warnings...) but I made a wrong manipulation during the
diff (I wasn't in the rigth directory).


> I think it would be a lot clearer if the sense of the
> return value from `MR_trace_get_type_filtered' was
> the same as that of `MR_trace_get_type', rather than being
> the opposite, as it is in your change.

I agree.

> 
> Can I see another relative diff this time?



--- mercury_trace_external.c.save	Wed Jul 29 15:10:43 1998
--- mercury_trace_external.c.save	Wed Jul 29 15:10:43 1998
+++ mercury_trace_external.c	Wed Jul 29 16:05:59 1998
@@ -54,9 +55,11 @@
 	MR_REQUEST_ABORT_PROG    = 5, /* abort the current execution	      */
 	MR_REQUEST_ERROR         = 6, /* something went wrong                
*/
 	MR_REQUEST_CURRENT_LIVE_VAR_NAMES  
-				 = 7, /* report data for current_live_var_names query */
+				 = 7, /* report data for 
+					 current_live_var_names query */
 	MR_REQUEST_CURRENT_NTH_VAR 
-				 = 8  /* report data for current_nth_var query */
+				 = 8  /* report data for 
+					 current_nth_var query */
 
 } MR_debugger_request_type;
 
@@ -79,7 +82,8 @@
 static void	MR_output_current_nth_var(Word var);
 static void	MR_output_current_live_var_names(Word var_names_list, 
 						 Word type_list);
-static Word	MR_trace_make_var_names_list(const MR_Stack_Layout_Label
*layout);
+static Word	MR_trace_make_var_names_list(
+			const MR_Stack_Layout_Label *layout);
 static Word	MR_trace_make_type_list(const MR_Stack_Layout_Label
*layout);
 static Word	MR_trace_make_nth_var(const MR_Stack_Layout_Label *layout, 
 				      Word debugger_request);
@@ -366,9 +370,11 @@
 			case MR_REQUEST_CURRENT_LIVE_VAR_NAMES:
 				if (MR_debug_socket) {
 					fprintf(stderr, "\nMercury runtime: "
-						"MR_REQUEST_CURRENT_LIVE_VAR_NAMES\n");
+						"MR_REQUEST_CURRENT_LIVE_VAR"
+						"_NAMES\n");
 				}
-				var_names_list = MR_trace_make_var_names_list(layout);
+				var_names_list = 
+				  MR_trace_make_var_names_list(layout);
 				type_list = MR_trace_make_type_list(layout);
 				MR_output_current_live_var_names(var_names_list,
 								 type_list);
@@ -380,8 +386,10 @@
 						"REQUEST_CURRENT_VARS\n");
 				}
 				var_list = MR_trace_make_var_list(layout);
-				var_names_list = MR_trace_make_var_names_list(layout);
-				MR_output_current_vars(var_list, var_names_list);
+				var_names_list = 
+				  MR_trace_make_var_names_list(layout);
+				MR_output_current_vars(var_list, 
+						       var_names_list);
 				break;
 
 			case MR_REQUEST_CURRENT_NTH_VAR:
@@ -568,10 +576,7 @@
 		name = MR_name_if_present(vars, i);
 		var = &vars->MR_slvs_pairs[i];
 
-		if ((strncmp(name, "TypeInfo", 8) == 0)
-		|| (strncmp(name, "ModuleInfo", 10) == 0)
-		|| (strncmp(name, "HLDS", 4) == 0)
-		|| !MR_trace_get_type(var, NULL, &type_info))
+		if (!MR_trace_get_type_filtered(var, name, &type_info))
 		{
 			continue;
 		}
@@ -591,7 +596,8 @@
 */
 
 static Word
-MR_trace_make_nth_var(const MR_Stack_Layout_Label *layout, Word
debugger_request)
+MR_trace_make_nth_var(const MR_Stack_Layout_Label *layout, 
+		      Word debugger_request)
 {
 	int 				var_number;
 	const MR_Stack_Layout_Vars 	*vars;
@@ -604,7 +610,8 @@
 	Word				univ;
 
 	var_number = MR_get_var_number(debugger_request);
-		/* debugger_request should be of the form:
current_nth_var(var_number) */
+		/* debugger_request should be of the form: 
+		   current_nth_var(var_number) */
 	vars = &layout->MR_sll_var_info;
 	name = MR_name_if_present(vars, var_number);
 	var = &vars->MR_slvs_pairs[var_number];
@@ -612,16 +619,12 @@
 	restore_transient_registers();
 	incr_hp(univ, 2);
 
-	if ((strncmp(name, "TypeInfo", 8) == 0)
-	    || (strncmp(name, "ModuleInfo", 10) == 0)
-	    || (strncmp(name, "HLDS", 4) == 0)
-	    || !MR_trace_get_type_and_value(var, NULL, &type_info, &value))
+
+	if (MR_trace_get_type_and_value_filtered(var, name, &type_info, 
+						     &value))
 	  {
 	    field(mktag(0), univ, UNIV_OFFSET_FOR_TYPEINFO) = type_info;
 	    field(mktag(0), univ, UNIV_OFFSET_FOR_DATA) = value;
-	  } else {
-	    field(mktag(0), univ, UNIV_OFFSET_FOR_TYPEINFO) = type_info;
-	    field(mktag(0), univ, UNIV_OFFSET_FOR_DATA) = value;
 	  }
 
 	save_transient_registers();
@@ -640,9 +643,6 @@
 static int
 MR_get_var_number(Word debugger_request)
 {
-	int result;
-
-	result = MR_DI_get_var_number(debugger_request);
-	return result;
+	return MR_DI_get_var_number(debugger_request);
 }
 #endif /* MR_USE_EXTERNAL_DEBUGGER */


--- mercury_trace_util.c.save	Wed Jul 29 14:55:21 1998
+++ mercury_trace_util.c	Wed Jul 29 16:33:45 1998
@@ -150,15 +150,10 @@
 		** XXX The printing of large data structures is painful
 		** at the moment due to the lack of a true browser.
- 		**
-		** "variables" representing the saved values of succip, hp etc,
-		** which are the "variables" for which get_type_and_value
-		** fails, are not of interest to the trace analyzer.
 		*/
 
-		if ((strncmp(name, "TypeInfo", 8) == 0)
-		|| (strncmp(name, "ModuleInfo", 10) == 0)
-		|| (strncmp(name, "HLDS", 4) == 0)
-		|| !MR_trace_get_type_and_value(var, NULL, &type_info, &value))
+		if (!MR_trace_get_type_and_value_filtered(var, name, 
+							  &type_info, &value))
 		{
 			continue;
 		}
@@ -371,3 +366,34 @@
 }
 
 
+
+
+/*
+** "variables" representing the saved values of succip, hp etc,
+** which are the "variables" for which get_type_and_value
+** fails, are not of interest to the trace analyzer.
+** MR_trace_get_type_and_value_filtered() and
MR_trace_get_type_filtered()
+** will succeed for theses variables.
+*/
+
+bool
+MR_trace_get_type_and_value_filtered(const MR_Stack_Layout_Var *var, 
+				     const char *name, 
+				     Word *type_info, Word *value)
+{
+	return ((strncmp(name, "TypeInfo", 8) != 0)
+	       && (strncmp(name, "ModuleInfo", 10) != 0)
+	       && (strncmp(name, "HLDS", 4) != 0)
+	       && MR_trace_get_type_and_value(var, NULL, type_info, value));
+}
+
+
+bool
+MR_trace_get_type_filtered(const MR_Stack_Layout_Var *var, 
+			   const char *name, Word *type_info)
+{
+	return ((strncmp(name, "TypeInfo", 8) != 0)
+	       && (strncmp(name, "ModuleInfo", 10) != 0)
+	       && (strncmp(name, "HLDS", 4) != 0)
+	       && MR_trace_get_type(var, NULL, type_info));
+}


--- mercury_trace_util.h.save	Wed Jul 29 15:46:08 1998
+++ mercury_trace_util.h	Wed Jul 29 15:49:48 1998
@@ -32,6 +32,11 @@
 extern	bool	MR_trace_get_type_base(const MR_Stack_Layout_Var *var,
 			bool saved_regs_valid, Word *base_sp, Word *base_curfr,
 			Word *type_params, Word *type_info);
+extern	bool	MR_trace_get_type_and_value_filtered(
+			const MR_Stack_Layout_Var *var, const char *name, 
+			Word *type_info, Word *value);
+extern	bool	MR_trace_get_type_filtered(const MR_Stack_Layout_Var *var, 
+			const char *name, Word *type_info);
 
 
 /*

-- 
R1.