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