On Thu, Mar 21, 2013 at 12:02 AM, Janne Blomqvist <blomqvist.ja...@gmail.com> wrote: > Thanks for the prompt review! > > On Tue, Mar 19, 2013 at 7:30 PM, Tobias Burnus <bur...@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 <j...@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
