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

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

[Janne Blomqvist]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Hi,

now that the Fortran frontend is C++ we can use the primitive bool
type instead of inventing our own. The patch is almost entirely
mechanical, apart from manually removing the definition of enum
gfc_try in gfortran.h, it has been generated with the following
script:

#!/bin/sh

for file in $(ls -1 *.c *.h)
do
    sed 's/FAILURE/false/' $file > tmp
    sed 's/SUCCESS/true/' tmp > tmp2
    sed 's/gfc_try/bool/' tmp2 > tmp
    mv tmp $file
    rm tmp2
done

Regtested on x86_64-unknown-linux-gnu. I get an XPASS on
gfortran.dg/do_1.f90, but IIRC I have encountered this before so it
should not be caused by this patch. Ok for trunk (4.9)?

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

[-- Attachment #2: boolfront.diff.gz --]
[-- Type: application/x-gzip, Size: 121497 bytes --]

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

[Tobias Burnus]

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. The advantage of 
FAILURE and SUCCESS is that they immediately make clear whether some 
call was successful or not. "true" and "false" don't.

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

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.

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.

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

[Janne Blomqvist]

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

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

[Janne Blomqvist]

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

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

[N.M. Maclaren]

On Mar 19 2013, Tobias Burnus 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. ...

Er, that is making a serious mistake or, at least, running the risk of
one.  C++'s bool and C99's _Bool are NOT compatible types.  The UK tried
to get _Bool either made compatible with C++ or (preferably) dropped, but
failed in both.

Now, in disciplined code that does not pass such objects between C and
C++, you won't see the differences.  But, if EITHER assertion fails,
there are some very nasty gotchas.


Regards,
Nick Maclaren.

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

[Joseph S. Myers]

On Thu, 21 Mar 2013, N.M. Maclaren wrote:

> On Mar 19 2013, Tobias Burnus 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. ...
> 
> Er, that is making a serious mistake or, at least, running the risk of
> one.  C++'s bool and C99's _Bool are NOT compatible types.  The UK tried
> to get _Bool either made compatible with C++ or (preferably) dropped, but
> failed in both.

They have been ABI-compatible in GCC ever since I implemented C99 _Bool 
(as opposed to the different, incompatible version in earlier C9X drafts) 
in <http://gcc.gnu.org/ml/gcc-patches/2000-10/msg01127.html>, and the 
semantics are essentially the same except that C++ disallows decrement 
operators (prefix and postfix --) on bool and C allows them.  (I don't 
know what C++ specifies regarding bool bit-fields or whether there are any 
incompatibilities there, but for non-bit-field objects you shouldn't have 
problems.)

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Janne Blomqvist]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Thu, Mar 21, 2013 at 11:31 PM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> Updated patch which in addition does the above transformations as
> well.

.. and here is the actual patch (thanks Bernhard!)


-- 
Janne Blomqvist

[-- Attachment #2: boolfront2.diff.gz --]
[-- Type: application/x-gzip, Size: 128147 bytes --]

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

[N.M. Maclaren]

On Mar 21 2013, Joseph S. Myers wrote:
>> > 
>> > > 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. ...
>> 
>> Er, that is making a serious mistake or, at least, running the risk of
>> one.  C++'s bool and C99's _Bool are NOT compatible types.  The UK tried
>> to get _Bool either made compatible with C++ or (preferably) dropped, but
>> failed in both.
>
>They have been ABI-compatible in GCC ever since I implemented C99 _Bool 
> ....

That is another matter entirely.  The code of gcc/gfortran is supposed
to be compilable with other compilers, and it is foolish to make
unnecessary assumptions by relying on undefined behaviour.

The simple facts are that C++ does NOT define bool to be compatible with
_Bool, and there are enough differences in specification and semantics
to make it likely that they are not the same in at least some compilers.
This mess has caused and still causes no end of problems for Fortran
interoperability and in standards like MPI.


Regards,
Nick Maclaren.

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

[Miles Bader]

"N.M. Maclaren" <nmm1@cam.ac.uk> writes:
> That is another matter entirely.  The code of gcc/gfortran is supposed
> to be compilable with other compilers, and it is foolish to make
> unnecessary assumptions by relying on undefined behaviour.
>
> The simple facts are that C++ does NOT define bool to be compatible with
> _Bool, and there are enough differences in specification and semantics
> to make it likely that they are not the same in at least some compilers.
> This mess has caused and still causes no end of problems for Fortran
> interoperability and in standards like MPI.

Er, maybe so, but is there actually an issue in _this_ case?

From original message, it sounds like these are internal compiler
interfaces, so all that matters is that they be self-consistent, and
there's no external ABI involved... So as long as all the code in
question is compiled with C++ and uses C++ bool, or is compiled with
C99 and uses _Bool, and doesn't use any functionality that isn't in
the intersection of those two types, there shouldn't be any problem,
right?

-miles

-- 
Congratulation, n. The civility of envy.

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

[N.M. Maclaren]

On Mar 22 2013, Miles Bader wrote:
>
>> That is another matter entirely.  The code of gcc/gfortran is supposed
>> to be compilable with other compilers, and it is foolish to make
>> unnecessary assumptions by relying on undefined behaviour.
>>
>> The simple facts are that C++ does NOT define bool to be compatible with
>> _Bool, and there are enough differences in specification and semantics
>> to make it likely that they are not the same in at least some compilers.
>> This mess has caused and still causes no end of problems for Fortran
>> interoperability and in standards like MPI.
>
>Er, maybe so, but is there actually an issue in _this_ case?

Yes.  There are many ways of resolving it, one of which is to state that
gfortran MUST be compiled in a second pass using gcc, after having built
gcc.  Another is described below.

From original message, it sounds like these are internal compiler
>interfaces, so all that matters is that they be self-consistent, and
>there's no external ABI involved... So as long as all the code in
>question is compiled with C++ and uses C++ bool, or is compiled with
>C99 and uses _Bool, and doesn't use any functionality that isn't in
>the intersection of those two types, there shouldn't be any problem,
>right?

And that is precisely the point.  The risk isn't in either of them,
but in any interface that passes C++ bool from the front-end to C99
_Bool in the back end.  If you exclude that, there is no problem.



This is a symptom of a much wider and more serious malaise, which
I do NOT think should be discussed in this thread.  I am going to
mention it only if someone wants to contact me about it by Email,
or start a new thread.

There are much more and fouller gotchas to do with C++ and C99.
complex is worse, because there are serious differences in the
alignment requirements, and I know that affects some systems quite
badly.  But the real nasties are to do with <cmath> and errno and,
worst of all, the horrors of C99 Annex F.

My point here is that it is VERY important not to read the political
sound-bite that C++ is now based on C99 and assume that they are
actually compatible in all respects where they have 'equivalent'
facilities.  They aren't.


Regards,
Nick Maclaren.

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

[Tobias Burnus]

N.M. Maclaren wrote:
> On Mar 22 2013, Miles Bader wrote:
>>
>>> That is another matter entirely.  The code of gcc/gfortran is supposed
>>> to be compilable with other compilers, and it is foolish to make
>>> unnecessary assumptions by relying on undefined behaviour.
>>>
>>> The simple facts are that C++ does NOT define bool to be compatible 
>>> with
>>> _Bool, and there are enough differences in specification and semantics
>>> to make it likely that they are not the same in at least some 
>>> compilers.
>>> This mess has caused and still causes no end of problems for Fortran
>>> interoperability and in standards like MPI.
>>
>> Er, maybe so, but is there actually an issue in _this_ case?
>
> Yes.  There are many ways of resolving it, one of which is to state that
> gfortran MUST be compiled in a second pass using gcc, after having built
> gcc.  Another is described below.

Well, the recommend way to compile GCC is to bootstrap it. That uses the 
system compiler (or whatever the user has) to compile GCC's C and C++ 
compilers; those are then used to compile GCC (this time including the 
Fortran front end) - for the compiler: twice, once without (stage 2) and 
once with (stage 3) debugging symbols. Then, the generated code is 
compared and shall not differ.

Users can compile GCC also without bootstrapping. However, I also do not 
see problems: Older GCC are compiled by C, newer by C++. But C and C++ 
are not mixed and this "bool" is really only used internally in the 
compiler and internally in the libgfortran run-time library (the latter 
is always compiled by GCC 's just created C compiler).

Thus, I do not see how this can cause problems.

That's different to user code, where mixing Fortran's LOGICAL, C's and 
C++'s bool can cause problems - as can passing an C or C++'s "int" to a 
Fortran LOGICAL (or without prototype: To C/C++'s bool). Or using 
different Fortran compilers with LOGICAL. I concur that this is an 
underestimated problem, but I fail to see how it applys here.


> And that is precisely the point.  The risk isn't in either of them,
> but in any interface that passes C++ bool from the front-end to C99
> _Bool in the back end.  If you exclude that, there is no problem.

The front end and the backend are both compiled with the same compiler 
and in the same binary. Even without bootstrapping, trying to compile 
them with different compilers, will require some heavy editing of 
makefiles. Thus, it seems to be extremely unlikely that different 
compilers have ever been used.  The change from C99's bool to C++'s bool 
occurred for the whole compiler. Before 4.7 the default was to compile 
GCC with C (though it had been modified to be also compilable with C++); 
in 4.7, the default compiler was C++ but C was still supported. And 
since GCC 4.8 C++ features are used, which makes GCC only compilable 
with C++. Actually, the FE itself so far uses nearly no C++ features, it 
is the middle end which used them (which 'leak' to the front end via 
some helper macros/functions).


> My point here is that it is VERY important not to read the political
> sound-bite that C++ is now based on C99 and assume that they are
> actually compatible in all respects where they have 'equivalent'
> facilities.  They aren't.

In terms of the discussion, "bool" is used for quite some time in 
gfortran (the frontend) and libgfortran. When GCC was compiled with C, 
that was C99's _Bool. Since the compiler is compiled as C++, it is C++'s 
"bool". Thus, C/C++'s bool are never mixed.

Tobias

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

[N.M. Maclaren]

On Mar 22 2013, Tobias Burnus wrote:
>
>The front end and the backend are both compiled with the same compiler 
>and in the same binary. Even without bootstrapping, trying to compile 
>them with different compilers, will require some heavy editing of 
>makefiles. Thus, it seems to be extremely unlikely that different 
>compilers have ever been used.  The change from C99's bool to C++'s bool 
>occurred for the whole compiler. Before 4.7 the default was to compile 
>GCC with C (though it had been modified to be also compilable with C++); 
>in 4.7, the default compiler was C++ but C was still supported. And 
>since GCC 4.8 C++ features are used, which makes GCC only compilable 
>with C++. Actually, the FE itself so far uses nearly no C++ features, it 
>is the middle end which used them (which 'leak' to the front end via 
>some helper macros/functions).

Ah.  I misunderstood what people said.  Yes, if C++ mode is used for
the whole compiler, everything is fine.  And it remains so even if
the code plays fairly clever games.  I don't know if there are any
compilers that generate incompatible bool types in C and C++ modes,
but it wouldn't surprise me if there were.

>In terms of the discussion, "bool" is used for quite some time in 
>gfortran (the frontend) and libgfortran. When GCC was compiled with C, 
>that was C99's _Bool. Since the compiler is compiled as C++, it is C++'s 
>"bool". Thus, C/C++'s bool are never mixed.

Then there is no problem.  Sorry about the red herring - I misunderstood
what people said.  Unfortunately, from personal experience, the majority 
of people even on WG14 and WG21 do not understand the incompatibilities,
which is why I posted.


Regards,
Nick Maclaren.

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

[Janne Blomqvist]

PING (now in plain text mode so that the lists will accept the
message, hopefully. $#%& gmail "improvements".)


On Fri, Mar 22, 2013 at 8:58 AM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
>
> On Thu, Mar 21, 2013 at 11:31 PM, Janne Blomqvist
> <blomqvist.janne@gmail.com> wrote:
> > Updated patch which in addition does the above transformations as
> > well.
>
> .. and here is the actual patch (thanks Bernhard!)
>
>
> --
> Janne Blomqvist




--
Janne Blomqvist

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

[Tobias Burnus]

Janne Blomqvist wrote:
> On Thu, Mar 21, 2013 at 11:31 PM, Janne Blomqvist
> <blomqvist.janne@gmail.com> wrote:
>> Updated patch which in addition does the above transformations as
>> well.
> .. and here is the actual patch (thanks Bernhard!)
http://gcc.gnu.org/ml/fortran/2013-03/msg00108.html

Thanks for the update and sorry for the delay. The patch idea as such is 
okay. However, the patch isn't.


+         if (!gfc_notify_std(GFC_STD_F2003, "Noninteger ""exponent in 
an initialization ""expression at %L", &op2->where))

Missing " " before the "(" additionally, the line is way too long. 
That's actually an issue throughout the whole file.

Additionally, the reformating caused code like:  "Noninteger ""exponent 
in  ....    The "" is quite ugly.


If you fix those issues, and update the patch for the newly added code 
(which presumably added a few FAILUREs), the patch is okay.

It is, indeed, most of the time helpful as it shortens the code without 
loosing its clearness. (Only at a few places, 'I found FAILURE/SUCCESS a 
tad clearer.)

Thanks for the patch.


For nicer looking code, you could also do:

* Remove the trailing " " for
+           return false;
(That's the only modified line with a trailing space)

* Change
+  if (!gfc_resolve_expr(e)
+      || !gfc_specification_expr(e))
+    return false;
to
   if (!gfc_resolve_expr(e) || !gfc_specification_expr(e))

* Ditto for:
+         if (t
               && b->expr1 != NULL
and a few more.


Tobias

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

[Mikael Morin]

Le 08/04/2013 10:34, Tobias Burnus a écrit :
> Janne Blomqvist wrote:
>> On Thu, Mar 21, 2013 at 11:31 PM, Janne Blomqvist
>> <blomqvist.janne@gmail.com> wrote:
>>> Updated patch which in addition does the above transformations as
>>> well.
>> .. and here is the actual patch (thanks Bernhard!)
> http://gcc.gnu.org/ml/fortran/2013-03/msg00108.html
> 
> Thanks for the update and sorry for the delay. The patch idea as such is
> okay. However, the patch isn't.
> 
[... formatting problems ...]

there is also a SUCCESS_EXIT_CODE changed to true_EXIT_CODE.
I think that's unintended.

Mikael

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

[Janne Blomqvist]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Mon, Apr 8, 2013 at 11:34 AM, Tobias Burnus <burnus@net-b.de> wrote:
> Janne Blomqvist wrote:
>>
>> On Thu, Mar 21, 2013 at 11:31 PM, Janne Blomqvist
>> <blomqvist.janne@gmail.com> wrote:
>>>
>>> Updated patch which in addition does the above transformations as
>>> well.
>>
>> .. and here is the actual patch (thanks Bernhard!)
>
> http://gcc.gnu.org/ml/fortran/2013-03/msg00108.html
>
> Thanks for the update and sorry for the delay. The patch idea as such is
> okay. However, the patch isn't.
>
>
> +         if (!gfc_notify_std(GFC_STD_F2003, "Noninteger ""exponent in an
> initialization ""expression at %L", &op2->where))
>
> Missing " " before the "(" additionally, the line is way too long. That's
> actually an issue throughout the whole file.
>
> Additionally, the reformating caused code like:  "Noninteger ""exponent in
> ....    The "" is quite ugly.
>
>
> If you fix those issues, and update the patch for the newly added code
> (which presumably added a few FAILUREs), the patch is okay.
>
> It is, indeed, most of the time helpful as it shortens the code without
> loosing its clearness. (Only at a few places, 'I found FAILURE/SUCCESS a tad
> clearer.)
>
> Thanks for the patch.
>
>
> For nicer looking code, you could also do:
>
> * Remove the trailing " " for
> +           return false;
> (That's the only modified line with a trailing space)
>
> * Change
> +  if (!gfc_resolve_expr(e)
> +      || !gfc_specification_expr(e))
> +    return false;
> to
>   if (!gfc_resolve_expr(e) || !gfc_specification_expr(e))
>
> * Ditto for:
> +         if (t
>               && b->expr1 != NULL
> and a few more.
>
>
> Tobias

Thanks for the feedback. The updated patch I committed is attached. It
should fix most of the formatting issues as well, although I may have
missed some. It also fixes the issue pointed out by Mikael.

--
Janne Blomqvist

[-- Attachment #2: boolfront3.diff.gz --]
[-- Type: application/x-gzip, Size: 131165 bytes --]