This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, Mar 21, 2013 at 12:02 AM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> 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.

Updated patch which in addition does the above transformations as
well. In order to do these, I used the coccinelle with the semantic
patch:

@@
expression E;
@@
(
- E == FAILURE
+ !E
)

@@
expression E;
@@
(
- E == SUCCESS
+ E
)

@@
expression E;
@@
(
- E != SUCCESS
+ !E
)

@@
expression E;
@@
(
- E != FAILURE
+ E
)

Regtested on x86_64-unknown-linux-gnu, Ok for trunk?

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



-- 
Janne Blomqvist


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]