[m-rev.] for review: --disable-deep-profiler

Fergus Henderson fjh at cs.mu.OZ.AU
Tue Jun 5 23:46:58 AEST 2001

On 05-Jun-2001, Peter Ross <peter.ross at miscrit.be> wrote:

> +# Do we disable building of the deep profiler.

The comment here should say what the allowed values are (`yes' or `no').

It would be slightly better to have the variable be the positive sense
(i.e. ENABLE_DEEP_PROFILER) rather than the negative sense.
In general using positive variable names rather than negative ones
avoids double negations, which makes things easier to understand.

Apart from anything else, you'd save a few lines of code:

> +# Add an option that disables the deep profiler.
> +#
> +AC_ARG_ENABLE(deep-profiler,
> +	[  --disable-deep-profiler disable the deep profiler],
> +	mercury_cv_enable_deep_profiler="$enableval",
> +	mercury_cv_enable_deep_profiler=yes
> +)
> +if test "$mercury_cv_enable_deep_profiler" = "no"; then
> +else
> +fi

The five lines in the middle could be replaced by just


Also, the deep profiler should not be enabled by default; it should
only be enabled by default on platforms that support the Posix features
that it uses.  If there isn't code yet to check for those features,
then I think it would be better to leave it disabled by default.

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-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe

More information about the reviews mailing list