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

Peter Ross peter.ross at miscrit.be
Wed Jun 6 00:16:31 AEST 2001


I have taken your suggestions and disabled the deep profiler by default and
swapped the sense of the variable to ENABLE_DEEP_PROFILER.

Pete

Fergus wrote:
> On 05-Jun-2001, Peter Ross <peter.ross at miscrit.be> wrote:
>
> Mmake.common.in:
> > +# Do we disable building of the deep profiler.
> > +DISABLE_DEEP_PROFILER=@DISABLE_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
> > +    DISABLE_DEEP_PROFILER=yes
> > +else
> > +    DISABLE_DEEP_PROFILER=no
> > +fi
> > +AC_SUBST(DISABLE_DEEP_PROFILER)
>
> The five lines in the middle could be replaced by just
>
> ENABLE_DEEP_PROFILER=$mercury_cv_enable_deep_profiler
>
> 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
> --------------------------------------------------------------------------
>

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