headers
Tobias Burnus | 1 Sep 09:36
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Daniel Kraft wrote:
> Paul Richard Thomas wrote:
>> Your patch looks OK
Ditto from my side.

>> Why does the bit of code in module.c(load_needed)....
>>   mio_symbol (sym);
>>   sym->attr.use_assoc = 1;
>>   if (only_flag)
>>     sym->attr.use_only = 1;
>>   if (p->u.rsym.renamed)
>>     sym->attr.use_rename = 1;
>>
>>   return 1;
>>
>> not do the job?  Is it needed now, with your patch?
> If you want I can replace the second initialization (the one in your 
> snippet) by an assertion and see if the test-suite still passes and if 
> it does leave the assertion there.
I think one should do this: For the trunk, such a patch is OK.

For 4.3 I would wait a week or so and add then only the new line without 
replacing the old lines with the assert.

Tobias
Permalink | Reply | 14 comments |
headers
Paul Richard Thomas | 1 Sep 11:23
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Dear All,

>>
>> If you want I can replace the second initialization (the one in your
>> snippet) by an assertion and see if the test-suite still passes and if it
>> does leave the assertion there.
>
> I think one should do this: For the trunk, such a patch is OK.
>
> For 4.3 I would wait a week or so and add then only the new line without
> replacing the old lines with the assert.

Agreed.

>
> Tobias
>

Paul
Permalink | Reply | 0 comments |
headers
Paul Richard Thomas | 1 Sep 12:33
Picon

Re: [Patch, Fortran] Documentation for type-bound procedures

Daniel,

OK to commit

Many thanks for the rapid response.

Paul

On Sun, Aug 31, 2008 at 4:17 PM, Daniel Kraft <d <at> domob.eu> wrote:
> Daniel Kraft wrote:
>>
>> here's my follow-up patch to document type-bound procedures in a new
>> chapter about F2003 OOP internals in gfc-internals.texi and to extend
>> dump-parse-tree.c to output procedure bindings (type-bound procedures and
>> finalizers).  I also included code to output EXPR_COMPCALL and EXEC_COMPCALL
>> nodes; but as those are currently always transformed into ordinary calls
>> this code is never executed at the moment.  I still think it is best to put
>> it in and adapt it if needed later when those expressions will stay until
>> trans for dynamic dispatch (or whatever might come).
>
>>
>>
>> I'm of course happy about comments on my text on spelling or "more
>> English" sentences if you've got any :)
>
> Updated the patch to include the necessary changes for GENERIC bindings,
> too.  On the way I removed an unused member
> (gfc_expr->value.compcall->derived) that has survived my clean-up of the
> GENERIC patch.
>
(Continue reading)

Permalink | Reply | 2 comments |
headers
Daniel Kraft | 1 Sep 12:59
Picon

Re: [Patch, Fortran] Documentation for type-bound procedures

Paul Richard Thomas wrote:
> Daniel,
> 
> OK to commit

Committed as revision 139857.  Thanks for the review!

Daniel

> 
> Paul
> 
> On Sun, Aug 31, 2008 at 4:17 PM, Daniel Kraft <d <at> domob.eu> wrote:
>> Daniel Kraft wrote:
>>> here's my follow-up patch to document type-bound procedures in a new
>>> chapter about F2003 OOP internals in gfc-internals.texi and to extend
>>> dump-parse-tree.c to output procedure bindings (type-bound procedures and
>>> finalizers).  I also included code to output EXPR_COMPCALL and EXEC_COMPCALL
>>> nodes; but as those are currently always transformed into ordinary calls
>>> this code is never executed at the moment.  I still think it is best to put
>>> it in and adapt it if needed later when those expressions will stay until
>>> trans for dynamic dispatch (or whatever might come).
>>>
>>> I'm of course happy about comments on my text on spelling or "more
>>> English" sentences if you've got any :)
>> Updated the patch to include the necessary changes for GENERIC bindings,
>> too.  On the way I removed an unused member
>> (gfc_expr->value.compcall->derived) that has survived my clean-up of the
>> GENERIC patch.
>>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Daniel Kraft | 1 Sep 13:35
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Tobias Burnus wrote:
> Daniel Kraft wrote:
>> Paul Richard Thomas wrote:
>>> Your patch looks OK
> Ditto from my side.
> 
>>> Why does the bit of code in module.c(load_needed)....
>>>   mio_symbol (sym);
>>>   sym->attr.use_assoc = 1;
>>>   if (only_flag)
>>>     sym->attr.use_only = 1;
>>>   if (p->u.rsym.renamed)
>>>     sym->attr.use_rename = 1;
>>>
>>>   return 1;
>>>
>>> not do the job?  Is it needed now, with your patch?
>> If you want I can replace the second initialization (the one in your 
>> snippet) by an assertion and see if the test-suite still passes and if 
>> it does leave the assertion there.
> I think one should do this: For the trunk, such a patch is OK.

The assertion fails e.g. on use_only_1.f90.  I can only speculate here, 
but my interpretation is this:

load_needed() loads symbols that are "needed" for the ones USE'd (in 
use_only_1.f90, on USE'ing mod2::yfoobar (an INTERFACE) the procedures 
that are part of this interface are needed) and those can be symbols not 
listed in the USE...ONLY and thus the initialization there is necessary 
as is the one I added to get round the problem the PR was about.
(Continue reading)

Permalink | Reply | 10 comments |
headers
Paul Richard Thomas | 1 Sep 14:20
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Daniel,

> The assertion fails e.g. on use_only_1.f90.  I can only speculate here, but
> my interpretation is this:

I thought that this might be the case.

>
> load_needed() loads symbols that are "needed" for the ones USE'd (in
> use_only_1.f90, on USE'ing mod2::yfoobar (an INTERFACE) the procedures that
> are part of this interface are needed) and those can be symbols not listed
> in the USE...ONLY and thus the initialization there is necessary as is the
> one I added to get round the problem the PR was about.
>
> I've added a comment to my new line explaining why we need to initialize
> here and would suggest to take the patch as it is; but maybe you can come up
> with a better solution as you probably know the code better than I do.
>  Otherwise, ok to commit?

I think that we had better take the pragmatic view - it works.
"probably" is the operative word here - I wade into module.c every 6
months or so and have to learn afresh each time..... as I am doing
right now.

OK to commit.

Cheers

Paul
(Continue reading)

Permalink | Reply | 8 comments |
headers
Daniel Kraft | 1 Sep 15:49
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Paul Richard Thomas wrote:
> Daniel,
> 
>> The assertion fails e.g. on use_only_1.f90.  I can only speculate here, but
>> my interpretation is this:
> 
> I thought that this might be the case.
> 
>> load_needed() loads symbols that are "needed" for the ones USE'd (in
>> use_only_1.f90, on USE'ing mod2::yfoobar (an INTERFACE) the procedures that
>> are part of this interface are needed) and those can be symbols not listed
>> in the USE...ONLY and thus the initialization there is necessary as is the
>> one I added to get round the problem the PR was about.
>>
>> I've added a comment to my new line explaining why we need to initialize
>> here and would suggest to take the patch as it is; but maybe you can come up
>> with a better solution as you probably know the code better than I do.
>>  Otherwise, ok to commit?
> 
> I think that we had better take the pragmatic view - it works.
> "probably" is the operative word here - I wade into module.c every 6
> months or so and have to learn afresh each time..... as I am doing
> right now.
> 
> OK to commit.

Committed as revision 139866 to trunk.  If you want me to fix this on 
4.3, too, you'll have to tell me if there's something I should know for 
doing so :)  I think I'll have to check-out /branches/gcc-4_3-branch and 
do the fix there, too, right?  Anything special I should consider?
(Continue reading)

Permalink | Reply | 6 comments |
headers
Tobias Burnus | 1 Sep 20:15
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Daniel Kraft wrote:
> Committed as revision 139866 to trunk.  If you want me to fix this on 
> 4.3, too, you'll have to tell me if there's something I should know 
> for doing so :)  I think I'll have to check-out 
> /branches/gcc-4_3-branch and do the fix there, too, right?  Anything 
> special I should consider?
Nothing special, except that one should run "make check-gfortran" there 
as well. Some people write in the patch's changelog "Backport from 
mainline:" and then the date line from the mainline check in, but this 
is not needed (cf. "ChangeLog"s there). And usually we (= gfortraners) 
wait a week or so before checking into a branch to see whether a 
regression on the trunk occur. I think your patch is simple enough that 
one could do without the waiting, but you may prefer to wait still a few 
days.

Tobias
Permalink | Reply | 4 comments |
headers
Daniel Kraft | 1 Sep 20:19
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Tobias Burnus wrote:
> Daniel Kraft wrote:
>> Committed as revision 139866 to trunk.  If you want me to fix this on 
>> 4.3, too, you'll have to tell me if there's something I should know 
>> for doing so :)  I think I'll have to check-out 
>> /branches/gcc-4_3-branch and do the fix there, too, right?  Anything 
>> special I should consider?
> Nothing special, except that one should run "make check-gfortran" there 
> as well. Some people write in the patch's changelog "Backport from 
> mainline:" and then the date line from the mainline check in, but this 
> is not needed (cf. "ChangeLog"s there). And usually we (= gfortraners) 
> wait a week or so before checking into a branch to see whether a 
> regression on the trunk occur. I think your patch is simple enough that 
> one could do without the waiting, but you may prefer to wait still a few 
> days.

Thanks Tobias!  I already did a check-gfortran without regressions. 
I'll wait two days or so and check in after that; I'll post here for the 
check in and close the PR when done.

Yours,
Daniel

--

-- 
Done:     Arc-Bar-Cav-Sam-Val-Wiz, Dwa-Elf-Gno-Hum-Orc, Law-Neu-Cha, Fem-Mal
To go:    Hea-Kni-Mon-Pri-Ran-Rog-Tou
Permalink | Reply | 2 comments |
headers
Paul Richard Thomas | 1 Sep 20:35
Picon

Re: [Patch, Fortran] PR fortran/37193: USE m, ONLY: i, j => i

Daniel,

When you do it, would you kindly apply my patch for PR36371, please?
I still do not have the means to check anything in:-(  Trunk has been
patched for a good while with no problems.

Thanks

Paul

On Mon, Sep 1, 2008 at 8:19 PM, Daniel Kraft <d <at> domob.eu> wrote:
> Tobias Burnus wrote:
>>
>> Daniel Kraft wrote:
>>>
>>> Committed as revision 139866 to trunk.  If you want me to fix this on
>>> 4.3, too, you'll have to tell me if there's something I should know for
>>> doing so :)  I think I'll have to check-out /branches/gcc-4_3-branch and do
>>> the fix there, too, right?  Anything special I should consider?
>>
>> Nothing special, except that one should run "make check-gfortran" there as
>> well. Some people write in the patch's changelog "Backport from mainline:"
>> and then the date line from the mainline check in, but this is not needed
>> (cf. "ChangeLog"s there). And usually we (= gfortraners) wait a week or so
>> before checking into a branch to see whether a regression on the trunk
>> occur. I think your patch is simple enough that one could do without the
>> waiting, but you may prefer to wait still a few days.
>
> Thanks Tobias!  I already did a check-gfortran without regressions. I'll
> wait two days or so and check in after that; I'll post here for the check in
(Continue reading)

Permalink | Reply | 1 comment |

Gmane