[m-dev.] For Review: Bytecode interpreter

[Fergus Henderson]

On 25-Jan-2001, Levi Cameron <l.cameron2 at ugrad.unimelb.edu.au> wrote:
> Fergus Henderson wrote:
> 
> > > +EXTRA_MGNUCFLAGS= --no-ansi $(INC_DIRS)
> > 
> > `--no-ansi' should preferably not be used.
> > 
> > If it has to be used, then you should document exactly why it needs to
> > be used.
> 
> mb_disasm uses snprintf

That should be avoided, or at least enabled only conditionally,
since snprintf() is not portable.

> If I try MGNUCFLAGS-mb_disasm = --no-ansi
> mgnuc gets the --no-ansi option but then I get the error message:
> 
> cc1: Invalid option `-fno-ansi`

That's because you have some C compiler options (-I*) in MGNUCFLAGS
which should instead be in CFLAGS.  If you put them in CFLAGS instead,
that problem will go away.

> #-----------------------------------------------------------------------------#
> 
> MGNUCFLAGS	= --no-ansi $(MERCURY_INC)
> 
> MCFLAGS		= --generate-bytecode -O 0
> 
> C2INITFLAGS	= --trace
> MLFLAGS		= --trace
> MCFLAGS		= --trace shallow

There should be a comment here explaining why you need `--trace'.

> #-----------------------------------------------------------------------------#
> # The actual program (as distinct from bytecode interpreter)
> 
> HDRS		=
> 
> CFILES		= 
> 
> MFILES		= simple01.m
> 
> OBJS		= simple01_init.o $(MFILES:%.m=%.o) $(CFILES:%.c=%.o)
> 
> $(OBJS): $(HDRS) $(MB_HDRS)
> 
> #-----------------------------------------------------------------------------#
> ALL_HDRS	= $(HDRS) $(MB_HDRS)
> ALL_MFILES	= $(MFILES) $(MB_MFILES)
> ALL_CFILES	= $(CFILES) $(MB_CFILES)
> ALL_OBJS	= $(OBJS) $(MB_OBJS)
> 
> ALL_DEPENDS=$(ALL_MFILES:%=%.depend)
> 
> #-----------------------------------------------------------------------------#
> .PHONY: all
> all: test
> 
> test: $(ALL_OBJS)
> 	$(ML) $(ALL_GRADEFLAGS) $(ALL_MLFLAGS) $(ALL_OBJS) \
> 	-o test

It would be better to use the automatically generated rule for linking
`simple01' rather than hard-coding it here.
Just do

	ML_OBJS = $(MB_OBJS)

	.PHONY: test
	test: simple01

Or even better, link the MB_OBJS into a library libmer_bytecode.a
and use ML_LIBS = libmer_bytecode.a.

It might be better to make the target `check' rather than `test',
since `check' is the standard makefile target for running tests.

> clean_local: clean_o
> 
> realclean_local: realclean_o
> 
> .PHONY: clean_o
> clean_o:
> 	rm -f $(ALL_MFILES:%.m=%.mbc)
> 	rm -f $(ALL_MFILES:%.m=%.bytedebug)
> 	rm -f $(ALL_OBJS)
> 
> .PHONY: realclean_o
> realclean_o:
> 	rm -f tags test

Rather than using clean_o and realclean_o,
you should just use clean_local and realclean_local.
The extra level of indirection is unnecessary, and the name is misleading.
The `_o' original meant that this was cleaning up `.o'
files, but you're cleaning up other things too.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------