Hi Tobias,

> I do not see any reason why 'tmp' is replaced by 'temp' in this
> code. Also for doing patch archeology, it helps if there are no
> changes unless it makes sense. Adding an -e- does not count ;-)
> 
> Hence, OK with that change.

I've corrected that.  This also reduces the size of the patch.

> Regarding the change:
>   gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr)
> 
> It looks as if the pointer/check is done for
>    size(dt%foo%bar)
> for the 'bar' component' but shouldn't it also be
> done for each part ref, if it is a pointer/allocatable?
> (i.e. 'dt', 'foo' and 'bar'?
> 
> That's independent of the current patch.
> 
> Additionally, as there are a lot of special cases for
> CLASS – I wonder whether there also needs to be a special
> case for
>    size(dt%foo%class)
> ?
> 
> In particular, the following does ICE for me:

This ICEs even without any checks enabled and is now:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99585

> > P.S.: I couldn't find a Changelog entry that uses co-authors.  Is the
> > version
> > below correct?
> ...
> > Co-authored-by: Paul Thomas  <pa...@gcc.gnu.org>
> 
> I think you have two options: either the GIT way – as you did (although I 
> think the
> GIT way usually only has one and not two spaces before the email).
> 
> I did not see it in the commit logs, but it is used in the
> testcases for the change-log generator, see: 
> contrib/gcc-changelog/test_patches.txt

I've done it as described above.  In the git log I see the co-author, while

>   git show -1 | ./contrib/gcc-changelog/git_check_commit.py -p

produces:

Checking c2d7c39fcb8a3cb67600cdb6fde49ecb0e951589: OK
------ gcc/fortran/ChangeLog ------ 
2021-03-14  Harald Anlauf  <anl...@gmx.de>
            Paul Thomas  <pa...@gcc.gnu.org>

etc.  Nice!

Thanks for the review.  But as your testcase shows, we're not finished yet...

Thanks,
Harald

Reply via email to