[m-dev.] for review: introduction of two new directories

[Zoltan Somogyi]

> > +INSTALL_DOC_DIR 	= $(INSTALL_PREFIX)/lib/mercury/doc
> >  
> >  # Specify the Mercury compiler to use for bootstrapping
> >  MC			= @BOOTSTRAP_MC@
> 
> Should that be part of this change?

See below.

> >  .PHONY: compiler
> > -compiler: dep_compiler scripts util runtime boehm_gc library
> > +compiler: dep_compiler util scripts boehm_gc runtime library browser trace
> 
> Any particular reason for swapping the order of `util' and `scripts' there?

To be consistent with the other targets nearby. I wanted to make sure that
it is apparent that there is a satisfiable partial order of making the
directories.

> >  .PHONY: install_main
> >  install_main: all $(PREINSTALL_HACK) \
> >  		install_scripts install_util install_runtime install_boehm_gc 
       \
> > -  		install_library install_compiler install_profiler install_doc 
       \
> > +  		install_library install_browser install_trace \
> > +		install_compiler install_profiler install_doc \
> 
> Any particular reason for installing the `trace' library before
> installing the `browser' library?

You got the order reversed.

No reason, except for consistency with below. Does it matter?

> > +		( cd browser && \
> > +		  $(SUBDIR_MMAKE) GRADE=$$grade install_library ) && \
> > +		( cd trace && \
> > +		  $(SUBDIR_MMAKE) GRADE=$$grade install_lib ) && \
> 
> Likewise.

The trace directory depends on the browser directory, because e.g.
trace/mercury_trace_help.c imports the automatically generated file
browser/help.h.

> > However, since all Mercury code
> > +# in an executable must be of the same grade, we need to be able to
> > +# compile the modules in this directory in debug grades as well.
> 
> How about compiling this directory with `--trace minimal', then?
> That would minimize the overhead.

But not eliminate it. What we actually want is to compile it with *no* events
and no layouts (except possibly the minimal ones needed for stack tracing),
and yet still be able to claim that the grade is debug, so that it can be
linked into a debug grade program.

> If this is compiled with `--halt-at-warn', as it should be, then the
> warning will be treated as an error.

Yes, I agree this should be fixed. However, I don't think

> One possible solution would be to use nested modules:
> 
> 	:- module browser.
> 	:- interface.
> 	:- include_module help.

is a good solution. Really we need a mechanism for saying that a group of
modules without a main is a package. I recall there was a proposal for this,
but not what its status is.

> > Index: browser/help.m
> 
> Hmm, I'm wondering about the name "browser" now.
> Is this new directory supposed to contain a term browser, a help browser,
> or both?
> If the answer is "both", then "browsers" would be a better name.
> But it might be better still to use a more inclusive name, such
> as "debugger".

Both, but the term browser should eventually be significantly bigger
than the help browser. Hence the name.

> That shouldn't be part of this change, should it?

No, but this is not *a change*, it is only part of one. I only selected
file diffs; I did not tinker with each file diff.

> > Index: runtime/Mmakefile
> > @@ -45,6 +45,7 @@
> >  			mercury_imp.h		\
> >  			mercury_init.h		\
> >  			mercury_label.h		\
> > +			mercury_layout_util.h	\
> >  			mercury_library_types.h	\
> >  			mercury_memory.h	\
> >  			mercury_memory_zones.h	\
> > @@ -112,6 +109,7 @@
> >  			mercury_heap_profile.c	\
> >  			mercury_ho_call.c	\
> >  			mercury_label.c		\
> > +			mercury_layout_util.c	\
> >  			mercury_memory.c	\
> >  			mercury_memory_zones.c	\
> >  			mercury_memory_handlers.c	\
> 
> Should that be part of this change?

Ditto.

> > +# case $stack_trace,$require_tracing in
> > +# 	true,true)
> > +# 		use_trail=true ;;
> > +# esac
> 
> That should definitely not be part of this change.

Ditto. However, I have been waiting to see you in person to discuss that
change.

> >  case $mercury_libs in
> >  	shared)
> > -		LIBS=${MERCURY_LIBS="-lmercury -lmer $LIBGC $STDLIBS"}
> > +		# Repeating -l$STD_LIB_NAME for shared objects would have
> > +		# no effect.
> > +		LIBS=${MERCURY_LIBS="-l$TRACE_LIB_NAME -l$BROWSER_LIB_NAME -l$
       STD_LIB_NAME -l$RT_LIB_NAME $LIBGC $STDLIBS"}
> 
> Any particular reason why $BROWSER_LIB_NAME comes before $TRACE_LIB_NAME?

You got it backwards; it doesn't.

The trace library defines MR_trace, which calls the functions in
trace/mercury_trace_help.c. This includes references to help.m, defined
in the browser library. Later, the trace library will also call term browsers
from the browser library. Hence the order.

> > +	static)
> > +		# We repeat lib$STD_LIB_NAME.a. The first occurrence is to
> > +		# pick up objects needed by the program. Since the library
> > +		# may have been compiled with tracing even though the program
> > +		# isn't, this may then introduce references to MR_trace, which
> > +		# will pull in the browser library and possibly other objects
> > +		# from the standard library.
> 
> I thought the main purpose for introducing the two new directories
> was to avoid problems with cyclic dependencies between libraries.  But from
> the above, it sounds like there may be cyclic dependencies anyway, i.e.
> adding the new directories didn't solve the problem.  Is that correct?

There are two kinds of circularities: build order and reference order.
The second cannot be avoided as long as both the following are true:

(1) MR_trace calls Mercury code;
(2) this Mercury code calls anything in the Mercury standard library.

As long as it is possible for the Mercury standard library to call MR_trace,
these two create a circularity of reference, which is why you need the above
change. I don't think such circularities are much of a problem.

My change avoids the first kind of circularity, which are more problematic,
except for bootstrapping the compiler with tracing, which will probably need
a bit more work, but is a special case anyway. (My change should already
avoid circularities when we want to compile the library or the compiler
with an *installed* compiler and tracing library.)

> > Index: tools/bootcheck
> >  
> > +RT_LIB_NAME=mer_rt
> > +STD_LIB_NAME=mer_std
> > +TRACE_LIB_NAME=mer_trace
> > +BROWSER_LIB_NAME=mer_browser
> 
> I've seen those four lines of code in three places now.  I think you
> should at least add a comment in all 3 places saying something along
> the lines of "if you change this here, you may need to change it in
> these other two places".

This *was* done in one place; I will modify the other two.

> > +	# the *installed* pathname of mdb_doc and not its current pathname.
> > +	cat $root/doc/mdb_doc > $root/doc/test_mdbrc
> > +	sed -e '/^source/d' $root/doc/mdbrc >> $root/doc/test_mdbrc
> > +	MERCURY_DEBUGGER_INIT=$root/doc/test_mdbrc
> > +	export MERCURY_DEBUGGER_INIT
> 
> Should that be part of this change?

Ditto.

> > +HDRS		=	\
> > +			mercury_macros.h		\
> > +			mercury_trace.h			\
> > +			mercury_trace_alias.h		\
> > +			mercury_trace_external.h 	\
> > +			mercury_trace_help.h		\
> > +			mercury_trace_internal.h	\
> > +			mercury_trace_spy.h		\
> > +			mercury_trace_tables.h
> > +
> > +# keep this list in alphabetical order, please
> > +CFILES		= 	\
> > +			mercury_trace.c			\
> > +			mercury_trace_alias.c		\
> > +			mercury_trace_external.c 	\
> > +			mercury_trace_help.c		\
> > +			mercury_trace_internal.c	\
> > +			mercury_trace_spy.c		\
> > +			mercury_trace_tables.c
> 
> The list of files added here doesn't match the list of files
> deleted from runtime/Mmakefile, I think.

That's right; several are new files. However, all files deleted from runtime
are added here, except one which is renamed in runtime itself.

> > +# the following header files are created automatically by Makefile.DLLs
> > +LIBMER_DLL_H	 = lib$(TRACE_LIB_NAME)_dll.h
> > +LIBMER_GLOBALS_H = lib$(TRACE_LIB_NAME)_globals.h
> > +
> > +include $(MERCURY_DIR)/Makefile.DLLs
> > +
> > +else
> > +
> > +DLL_CFLAGS =
> > +LIBMER_DLL_H =
> > +LIBMER_GLOBALS_H =
> > +DLL_DEF_LIB =
> > +
> > +endif
> 
> I think all the occurrences of `LIBMER' there should be something else.

OK, I have done s/LIBMER/LIB/ in {runtime,trace}/Mmakefile.

Zoltan.