Re: [Patch, fortran, 4.9] Use bool type instead gfc_try

[Janne Blomqvist <blomqvist dot janne at gmail dot com>]

Thanks for the prompt review!

On Tue, Mar 19, 2013 at 7:30 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Am 19.03.2013 13:15, schrieb Janne Blomqvist:
>
>> now that the Fortran frontend is C++ we can use the primitive bool
>> type instead of inventing our own.
>
>
> Well, C99's "bool" (_Bool) was already used before.

Yes; the patch is an attempt to clean out some old junk from the days
when the frontend was supposedly C89.

> The advantage of FAILURE
> and SUCCESS is that they immediately make clear whether some call was
> successful or not. "true" and "false" don't.

I think the difference is marginal at best, overshadowed by the fact
that true/false are built-in language literals that everyone is
familiar with. It's not like you're going to return false if the call
is successful (unless the function is named is_something_broken() or
such).

> Thus, one should consider whether some procedures should be renamed to make
> clear that true is successful and false is not.

Certainly, however, I think that issue exists regardless of whether
the return value is gfc_try or bool.

> For instance,
>     if (gfc_notify_standard (...) == FAILURE)
>       return MATCH_ERROR;
> becomes with your patch:
>     if (gfc_notify_standard (...) == false)
>       return MATCH_ERROR;
>
> I think a more logical expression would be
>     if (gfc_notify_standard (...))
>       return MATCH_ERROR;
> which removes the "== false" but also swaps true/false for the return value.
>
> There are probably some more cases. Without such a clean up, I fear that the
> code will become less readable than it is currently. While I think such
> changes can be done as follow up, I think committing the gfc_try -> bool
> patch only makes sense if you commit yourself to do such a cleanup.

I think it should be relatively straightforward to do a further
cleanup patch which would make the usage more idiomatic C/C++, i.e.
transformations like

x == true -> x
x == false -> !x
x != true -> !x
x != false -> x

where "x" is some boolean expression.

>
> Tobias
>
> PS: Generally, please wait with committals until GCC's web mail archive
> works again for gcc-cvs. That will hopefully be soon.
>
>
>> 2013-03-19  Janne Blomqvist  <jb@gcc.gnu.org>
>>
>>         * gfortran.h: Remove enum gfc_try, replace gfc_try with bool type.
>>         * arith.c: Replace gfc_try with bool type.
>>         * array.c: Likewise.
>>         * check.c: Likewise.
>>         * class.c: Likewise.
>>         * cpp.c: Likewise.
>>         * cpp.h: Likewise.
>>         * data.c: Likewise.
>>         * data.h: Likewise.
>>         * decl.c: Likewise.
>>         * error.c: Likewise.
>>         * expr.c: Likewise.
>>         * f95-lang.c: Likewise.
>>         * interface.c: Likewise.
>>         * intrinsic.c: Likewise.
>>         * intrinsic.h: Likewise.
>>         * io.c: Likewise.
>>         * match.c: Likewise.
>>         * match.h: Likewise.
>>         * module.c: Likewise.
>>         * openmp.c: Likewise.
>>         * parse.c: Likewise.
>>         * parse.h: Likewise.
>>         * primary.c: Likewise.
>>         * resolve.c: Likewise.
>>         * scanner.c: Likewise.
>>         * simplify.c: Likewise.
>>         * symbol.c: Likewise.
>>         * trans-intrinsic.c: Likewise.
>>         * trans-openmp.c: Likewise.
>>         * trans-stmt.c: Likewise.
>>         * trans-types.c: Likewise.



-- 
Janne Blomqvist