[m-dev.] for review: subdivision of mercury_trace.c

[Fergus Henderson]

On 15-Apr-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> > However, you could make mercury_trace_cmd.h a module of its own,
> > with mercury_trace_cmd.c containing only the definitions for the
> > global variables declared in mercury_trace_cmd.h.
> > This might be a better approach.
> 
> MR_trace() is declared in mercury_trace.h, and those types and
> variables are *used* in MR_trace() and nowhere else (although
> they are *assigned to* in other places), so having another module
> define them would go against my sense of style.  Also, this module
> would have only some global variables in it, with no code to use
> those variables, which is in itself bad. Hiding the variables
> and accessing them via function calls would cause unacceptable
> slowdown on what can be a very frequent event (calls to MR_trace).

Well, you could use macros instead of functions...
but hiding the accesses in get/set macros wouldn't buy you a lot.

Hmmm.  Communication via global variables is in general bad,
but that is how we're doing things currently.  A nicer solution
might be to have MR_trace_event(), MR_trace_event_internal()
and MR_trace_event_external() take a pointer to a command struct,
rather than setting global variables.  The command could thus be
a local static struct in MR_trace() rather than a set of global
variables.  Then mercury_trace_cmd.h would just define a struct type
rather than defining any global variables.

Comments?  I'm happy to make this change myself if you're happy with it.

> > ANSI C forbids source file which are empty after preprocessing.
> > This is a dumb restriction, but we should probably respect it.
> > I'm not sure what the best fix is -- perhaps just moving the
> > #includes out of the #ifdef.
> 
> We could always declare and define the exported functions,
> and then make their implementations abort if MR_USE_EXTERNAL_DEBUGGER
> is not defined. This way the space overhead will be minimal.
> Comments?

Sorry, I wasn't clear -- the restriction is only on .c files, not header
files.  So you don't need to modify the header files in this way.
I think just moving the #includes in the .c file out of the #ifdef
is probably the simplest fix.

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