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 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).
>
>
> 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 Fanfarillo<fanfarillo....@gmail.com>
>> Damian Rouson<dam...@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.000000000 +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
