[m-rev.] for review: coverage testing

[Ian MacLarty]

On Mon, Sep 18, 2006 at 02:55:25PM +1000, Zoltan Somogyi wrote:
> For review by anyone.
> 
> Zoltan.
> 
> Implement coverage testing. There is still at least one bug that prevents us
> from succcessfully coverage testing the compiler, but the system ought to work
> for smaller programs, and people have been asking for this capability.
> 
...
> 
> slice/mtc.m:

That should be "mct.m".

> 	Add this module, the code for the Mercury coverage test tool.
> 
> slice/mtc_diff.m:
> 	Add this module, the code for computing the diff between two trace
> 	counts files. The intended use is to compare two trace counts files
> 	dumped at different stages of execution. (Since foreign_procs can be
> 	used to invoke the C functions in the runtime tghat write out the trace

s/tghat/that

> 	counts files in the middle of a program's execution, not just the end.)
> 
> slice/mdice.m:
> slice/mslice.m:
> slice/mtc_union.m:
> 	Convert to four space indentation.
> 
> tools/bootcheck:
> 	Since the slice directory's grade is independent of the grade of the
> 	other directories, don't copy it to the stage2 and stage3 by default.
> 	If it is copied, then still compile it (and otherwise handle it)
> 	separate from the other directories.
> 
> 	Add an option for gathering coverage test data during bootchecking.
> 

Please could you document the mct and mtc_diff tools in the trace counts
section of the user guide.

Also could you add some test cases to the test suite?

...
> Index: runtime/mercury_trace_base.c
> ===================================================================
...
> +    if (summarize) {
> +        char        *cmd;
> +        int         cmd_len;
> +        int         status;
> +        int         i;
> +        const char  *old_options;
> +
> +        /* 30 bytes must be enough for the dot, the value of i, and space */
> +        len = strlen(MR_trace_count_summary_file) + 30;
> +        name = MR_malloc(len);
> +
> +        cmd_len = strlen(UNION_CMD) + 4;
> +        cmd_len += strlen(MR_trace_count_summary_file)
> +            + strlen(TEMP_SUFFIX) + 1;
> +        cmd_len += (MR_trace_count_summary_max + 1) * len;
> +
> +        cmd = MR_malloc(cmd_len);
> +
> +        strcpy(cmd, MR_trace_count_summary_cmd);
> +        strcat(cmd, " -o ");

You use UNION_CMD to determine the string length, but then copy
MR_trace_count_summary_cmd into the string.  This looks like a bug.

> +        strcat(cmd, MR_trace_count_summary_file);
> +        strcat(cmd, TEMP_SUFFIX);
> +        strcat(cmd, " ");
> +        strcat(cmd, MR_trace_count_summary_file);
> +        strcat(cmd, " ");
> +
> +        for (i = 1; i <= MR_trace_count_summary_max; i++) {
> +            snprintf(name, len, "%s.%d", MR_trace_count_summary_file, i);
> +            strcat(cmd, name);
> +            strcat(cmd, " ");
> +        }
> +
> +#if 0
> +        fp = fopen("/tmp/zs_summaries", "a");
> +        if (fp != NULL) {
> +            fprintf(fp, "%s\n", cmd);
> +            fclose(fp);
> +        }
> +#endif

The above looks like it should be deleted.

> +
> +        old_options = getenv("MERCURY_OPTIONS");
> +        if (old_options != NULL) {
> +            (void) setenv("MERCURY_OPTIONS", "", MR_TRUE);
> +            status = system(cmd);
> +            (void) setenv("MERCURY_OPTIONS", old_options, MR_TRUE);
> +        } else {
> +            status = system(cmd);
> +        }
> +
> +        if (status == 0) {
> +            strcpy(cmd, "mv ");
> +            strcat(cmd, MR_trace_count_summary_file);
> +            strcat(cmd, TEMP_SUFFIX);
> +            strcat(cmd, " ");
> +            strcat(cmd, MR_trace_count_summary_file);
> +            status = system(cmd);
> +
> +#if 0
> +            fp = fopen("/tmp/zs_summaries", "a");
> +            if (fp != NULL) {
> +                fprintf(fp, "%s\n", cmd);
> +                fclose(fp);
> +            }
> +#endif

This too.

...
> Index: runtime/mercury_trace_base.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_trace_base.h,v
> retrieving revision 1.53
> diff -u -b -r1.53 mercury_trace_base.h
> --- runtime/mercury_trace_base.h	14 Sep 2006 00:15:47 -0000	1.53
> +++ runtime/mercury_trace_base.h	14 Sep 2006 02:58:37 -0000
> @@ -117,11 +117,9 @@
>  
>  /*
>  ** For every label reachable from the module table, write the id of the label
> -** and the number of times it has been executed to trace counts file of this
> -** program, with the exception of labels that haven't been executed.
> -**
> -** The dummy argument allows this function to be registered with
> -** MR_register_exception_cleanup.
> +** and the number of times it has been executed to the specified file. For 
> +** labels that haven't been executed, write them out only if the coverage_test
> +** argument is true.
>  **
>  ** The file can be recognized as a Mercury trace counts file as its first
>  ** line matches MR_TRACE_COUNT_FILE_ID. The value of that macro should be
> @@ -130,7 +128,19 @@
>  
>  #define	MR_TRACE_COUNT_FILE_ID	    "Mercury trace counts file\n"
>  
> -extern	void	MR_trace_write_label_exec_counts_to_file(void *dummy);
> +extern	void		MR_trace_write_label_exec_counts(FILE *fp,
> +				MR_bool coverage_test);
> +
> +/*
> +** Figout out where (to which file) to write out the label execution counts,

s/Figout/Figure/

> Index: slice/mct.m
> ===================================================================
> +:- pred usage(io::di, io::uo) is det.
> +
> +usage(!IO) :-
> +    io.write_strings([
> +        "Usage: mct [-d] [-v] [-o output_file] file1 file2 ...\n",
> +        "The -d or --detailed option causes a report for each label\n",
> +        "that has not been executed, even if some other code has been\n",

You need to add "to be printed" after "executed".

Otherwise it looks okay.

Ian.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------