Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
2012/5/13 Tobias Burnus bur...@net-b.de: Committed as Rev. 187436. Thanks for the patch and congratulation to your first GCC contribution! Thank you for your priceless support! Alessandro
Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Tobias Burnus wrote: I think the ChangeLog is okay - as is the whole patch. I wouldn't mind if someone else (Paul? Janus?) could also glance at the patch; however, if there are no comments, I intent to commit the patch soon. Committed as Rev. 187436. Thanks for the patch and congratulation to your first GCC contribution! Tobias
Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Hello all,
in attachment there's the new candidate patch, revisited by Tobias,
for PR fortran/45170, PR fortran/52158 and PR fortran/49430
(unexpectedly).
Please, check the Changelog, I don't know whether the descriptions are ok.
The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.
gcc version 4.8.0 20120512 (experimental).
Have a nice weekend.
Alessandro.
2012/5/7 Tobias Burnus bur...@net-b.de:
Hello,
On 05/06/2012 09:37 PM, Alessandro Fanfarillo wrote:
The patch is bootstrapped and tested on x86_64-unknown-linux-gnu - gcc
version 4.8.0 20120506 (experimental)
2012-05-06 Alessandro Fanfarillofanfarillo@gmail.com
Paul Thomaspa...@gcc.gnu.org
Tobias Burnusbur...@net-b.de
PR fortran/52158
* resolve.c (resolve_fl_derived0): Add a new condition in the if
statement of the deferred-length character component error block.
* trans-expr (gfc_conv_procedure_call): Add new checks in the if
statement on component's attributes (regarding PR 45170).
Typo: trans-expr - trans-expr.c.
Well, either the second PR is related to the commit - then one should have
PR fortran/52158
PR fortran/45170
- or it is only that vaguely related that one should only mention it in the
patch submittal email.
As it fixes an ICE mentioned in PR 45170, I would add it. (I think one
should close that PR, create PRs for the remaining issues and possibly a
meta bug for tracking. That PR is really a mess.)
The description Add new checks in the if statement on component's
attributes is true but it makes the implicit assumption that the reader
knows that the commit is about ts.deferred. I had written something like
the following:
* resolve.c (resolve_fl_derived0): Allow TBPs with deferred-length
results.
* trans-expr (gfc_conv_procedure_call): Handle TBP with
deferred-length results.
I am sure one can write a nicer ChangeLog text, but at least it points to
TBP (type-bound procedures) and to functions which have
deferred-length-character result variable. (Actually, procedure-pointer
components are also affected.)
2012-05-06 Alessandro Fanfarillofanfarillo@gmail.com
Damian Rousondam...@rouson.net
PR fortran/45170
Typically, the bug reporters are only acknowledged via the PR itself (and
sometimes via a comment in the test case). That should be sufficient.
Additionally, you should quote the same PRs as in the actual patch
(fortran/ChangeLog). The test case tests that PR 52158 is fixed - and it
tests that the bug reported in comment 19 of PR 45170 is solved.
- if (ts.deferred (sym-attr.allocatable || sym-attr.pointer))
+ if (ts.deferred ((!comp (sym-attr.allocatable
+ || sym-attr.pointer)) || (comp (comp-attr.allocatable
+ || comp-attr.pointer
The spacing is wrong: You have a 14 before the || but you should have
1 tab followed by 6 spaces. (I don't think that this is a problem of the
email client as the if line still have a tab.)
Additionally, the line breaks have been wrongly placed; it should be:
+ if (ts.deferred
+ ((!comp (sym-attr.allocatable || sym-attr.pointer))
+ || (comp (comp-attr.allocatable ||
comp-attr.pointer
That way one sees more easily which belongs together. The is below
ts; the || is one to the right of the ( to which this part of the
expression belongs.
--- gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 1970-01-01
01:00:00.0 +0100
+++ gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
2012-05-06
19:26:29.498829273 +0200
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR fortran/45170
As above: You should also list PR fortran/52158.
As I realized when looking at the ChangeLog: Proc pointers are also
affected. I think one could append the following code to the test case,
which gives the same error without the patch. (Untested with the patch.)
module test
implicit none
type t
procedure(deferred_len), pointer, nopass :: ppt
end type t
contains
function deferred_len()
character(len=:), allocatable :: deferred_len
deferred_len = 'abc'
end function deferred_len
subroutine doIt()
type(t) :: x
x%ppt = deferred_len
if (abc /= x%ppt()) call abort ()
end subroutine doIt
end module test
use test
call doIt ()
end
Otherwise, the patch looks OK.
Thanks for the contribution - and congratulation to your first GCC patch.
Tobias
PS: For bean counters: There is a GCC copyright assignment entry for
Alessandro since 2012-04-18.
deferred_last.diff
Description: Binary data
Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Hello Alessandro, Alessandro Fanfarillo wrote: in attachment there's the new candidate patch, revisited by Tobias, for PR fortran/45170, PR fortran/52158 and PR fortran/49430 (unexpectedly). Please, check the Changelog, I don't know whether the descriptions are ok. The patch is bootstrapped and tested on x86_64-unknown-linux-gnu. Thanks for the patch. According to the source code, the diff was sent as Content-Type: application/octet-stream, which in principle makes reviewing more difficult. However, in my Thunderbird, it showed up as inline text - thus I am fine. I think the ChangeLog is okay - as is the whole patch. I wouldn't mind if someone else (Paul? Janus?) could also glance at the patch; however, if there are no comments, I intent to commit the patch soon. Tobias
Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Hello,
On 05/06/2012 09:37 PM, Alessandro Fanfarillo wrote:
The patch is bootstrapped and tested on x86_64-unknown-linux-gnu - gcc
version 4.8.0 20120506 (experimental)
2012-05-06 Alessandro Fanfarillofanfarillo@gmail.com
Paul Thomaspa...@gcc.gnu.org
Tobias Burnusbur...@net-b.de
PR fortran/52158
* resolve.c (resolve_fl_derived0): Add a new condition in the if
statement of the deferred-length character component error block.
* trans-expr (gfc_conv_procedure_call): Add new checks in the if
statement on component's attributes (regarding PR 45170).
Typo: trans-expr - trans-expr.c.
Well, either the second PR is related to the commit - then one should have
PR fortran/52158
PR fortran/45170
- or it is only that vaguely related that one should only mention it in
the patch submittal email.
As it fixes an ICE mentioned in PR 45170, I would add it. (I think one
should close that PR, create PRs for the remaining issues and possibly a
meta bug for tracking. That PR is really a mess.)
The description Add new checks in the if statement on component's
attributes is true but it makes the implicit assumption that the reader
knows that the commit is about ts.deferred. I had written something
like the following:
* resolve.c (resolve_fl_derived0): Allow TBPs with deferred-length
results.
* trans-expr (gfc_conv_procedure_call): Handle TBP with deferred-length
results.
I am sure one can write a nicer ChangeLog text, but at least it points
to TBP (type-bound procedures) and to functions which have
deferred-length-character result variable. (Actually, procedure-pointer
components are also affected.)
2012-05-06 Alessandro Fanfarillofanfarillo@gmail.com
Damian Rousondam...@rouson.net
PR fortran/45170
Typically, the bug reporters are only acknowledged via the PR itself
(and sometimes via a comment in the test case). That should be sufficient.
Additionally, you should quote the same PRs as in the actual patch
(fortran/ChangeLog). The test case tests that PR 52158 is fixed - and it
tests that the bug reported in comment 19 of PR 45170 is solved.
- if (ts.deferred (sym-attr.allocatable || sym-attr.pointer))
+ if (ts.deferred ((!comp (sym-attr.allocatable
+ || sym-attr.pointer)) || (comp (comp-attr.allocatable
+ || comp-attr.pointer
The spacing is wrong: You have a 14 before the || but you should
have 1 tab followed by 6 spaces. (I don't think that this is a problem
of the email client as the if line still have a tab.)
Additionally, the line breaks have been wrongly placed; it should be:
+ if (ts.deferred
+ ((!comp (sym-attr.allocatable || sym-attr.pointer))
+ || (comp (comp-attr.allocatable || comp-attr.pointer
That way one sees more easily which belongs together. The is below
ts; the || is one to the right of the ( to which this part of the
expression belongs.
--- gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 1970-01-01
01:00:00.0 +0100
+++ gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
2012-05-06
19:26:29.498829273 +0200
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR fortran/45170
As above: You should also list PR fortran/52158.
As I realized when looking at the ChangeLog: Proc pointers are also
affected. I think one could append the following code to the test case,
which gives the same error without the patch. (Untested with the patch.)
module test
implicit none
type t
procedure(deferred_len), pointer, nopass :: ppt
end type t
contains
function deferred_len()
character(len=:), allocatable :: deferred_len
deferred_len = 'abc'
end function deferred_len
subroutine doIt()
type(t) :: x
x%ppt = deferred_len
if (abc /= x%ppt()) call abort ()
end subroutine doIt
end module test
use test
call doIt ()
end
Otherwise, the patch looks OK.
Thanks for the contribution - and congratulation to your first GCC patch.
Tobias
PS: For bean counters: There is a GCC copyright assignment entry for
Alessandro since 2012-04-18.
Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Hi Alessandro, my name is Alessandro, I'm a newbie of GCC and helped by Tobias Burnus and Paul Thomas I'll try to add support for final subroutines. since Tobias already reviewed your patch, I just want to say: Welcome to the team :) We're certainly looking forward to your contributions! Having final subroutines in gfortran will be great progress. Btw, one thing that you may want to look into on your way to FINAL is proper deallocation of polymorphic variables(cf. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46321). This is one thing which is still missing in our OOP implementation, and it's serious in the sense that almost any sensible OOP program produces memory leaks with gfortran right now. It is not exactly a prerequisite to FINAL, but at least similar in spirit. Maybe both features can even be implemented with similar techniques. I actually had some plans to implement it myself, but currently I'm not able to spend large amounts of time on gfortran (unfortunately). In any case, I wish you all the best for your efforts ... Cheers, Janus
[Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7
Hello,
my name is Alessandro, I'm a newbie of GCC and helped by Tobias Burnus
and Paul Thomas I'll try to add support for final subroutines.
The patch is bootstrapped and tested on x86_64-unknown-linux-gnu - gcc
version 4.8.0 20120506 (experimental)
Best regards.
gcc/fortran/ChangeLog
2012-05-06 Alessandro Fanfarillo fanfarillo@gmail.com
Paul Thomas pa...@gcc.gnu.org
Tobias Burnus bur...@net-b.de
PR fortran/52158
* resolve.c (resolve_fl_derived0): Add a new condition in the if
statement of the deferred-length character component error block.
* trans-expr (gfc_conv_procedure_call): Add new checks in the if
statement on component's attributes (regarding PR 45170).
gcc/testsuite/ChangeLog
2012-05-06 Alessandro Fanfarillo fanfarillo@gmail.com
Damian Rouson dam...@rouson.net
PR fortran/45170
* gfortran.dg/deferred_type_param_3.f90: New.
Patch.diff
--- gcc-4.8/gcc/fortran/resolve.c 2012-05-06 19:29:21.794825508 +0200
+++ gcc-4.8-patched/gcc/fortran/resolve.c 2012-05-06 19:24:40.770831649
+0200
@@ -11666,7 +11666,7 @@
for ( ; c != NULL; c = c-next)
{
/* See PRs 51550, 47545, 48654, 49050, 51075 - and 45170. */
- if (c-ts.type == BT_CHARACTER c-ts.deferred)
+ if (c-ts.type == BT_CHARACTER c-ts.deferred !c-attr.function)
{
gfc_error (Deferred-length character component '%s' at %L is not
yet supported, c-name, c-loc);
diff -urN gcc-4.8/gcc/fortran/trans-expr.c
gcc-4.8-patched/gcc/fortran/trans-expr.c
--- gcc-4.8/gcc/fortran/trans-expr.c2012-05-06 19:29:21.878825505 +0200
+++ gcc-4.8-patched/gcc/fortran/trans-expr.c2012-05-06 19:25:53.134830069
+0200
@@ -4175,7 +4175,9 @@
we take the character length of the first argument for the result.
For dummies, we have to look through the formal argument list for
this function and use the character length found there.*/
- if (ts.deferred (sym-attr.allocatable || sym-attr.pointer))
+ if (ts.deferred ((!comp (sym-attr.allocatable
+ || sym-attr.pointer)) || (comp (comp-attr.allocatable
+ || comp-attr.pointer
cl.backend_decl = gfc_create_var (gfc_charlen_type_node, slen);
else if (!sym-attr.dummy)
cl.backend_decl = VEC_index (tree, stringargs, 0);
diff -urN gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
--- gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 1970-01-01
01:00:00.0 +0100
+++ gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
2012-05-06
19:26:29.498829273 +0200
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR fortran/45170
+!
+! Contributed by Damian Rouson
+
+module speaker_class
+ type speaker
+ contains
+procedure :: speak
+ end type
+contains
+ function speak(this)
+class(speaker) ,intent(in) :: this
+character(:) ,allocatable :: speak
+ end function
+ subroutine say_something(somebody)
+class(speaker) :: somebody
+print *,somebody%speak()
+ end subroutine
+end module
