Re: [PATCH] PR fortran/101536 - ICE in gfc_conv_expr_descriptor, at fortran/trans-array.c:7324
Hi Harald,
On 22.07.21 21:03, Harald Anlauf wrote:
you are right in that I was barking up the wrong tree.
I was focussed too much on the testcase in the PR.
[...]
Well, I tried and this does not work.
Which makes sense if one thinks about it:
When using 'a(5,:)', the parser already sets e->rank = 1.
while for 'a', the 'a' is the class wrapper with rank == 0 and
then overriding the e->rank by CLASS_DATA(e)->as.rank
+ adding AR_FULL makes sense.
However, an additional plain check on e->rank != 0 also in the
CLASS cases fixes the original issue as well as your example:
[...]
And regtests ok. :-)
See attached updated patch.
I think you still need to remove the 'return true;' from
the 'if (e->rank != 0 && e->ts.type == BT_CLASS' block – to
fall through to the e->rank check after the block.
(When 'return true;' is gone, the '{' and '}' can also be removed.)
Reason: Assume 'CLASS(...) x'. In this case, 'x' is a scalar.
And even after calling gfc_add_class_array_ref it remains
a scalar and e->rank == 0.
Or in other words: I think with your current patch,
class(u) :: z
f = size (z)
is wrongly accepted without an error.
Thus: OK with a scalar CLASS entry added which gives an error,
which I believe requires the removal of the 'return true;' line.
Thanks for the patch – and I find it surprising how many
combinations exist which all can go wrong.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Re: [PATCH] PR fortrsn/101564 - ICE in resolve_allocate_deallocate, at fortran/resolve.c:8169
Hi Harald,
On 22.07.21 21:50, Harald Anlauf wrote:
I am afraid we're really opening a can of worms here
which is not too bad if there are only two earthworms in there ;-)
Additionally, I wonder whether that will work with:
I think a "working" testcase for this could be:
program p
implicit none
integer, target :: ptr
integer, pointer :: A
allocate (A, stat=f())
print *, ptr
contains
function f()
integer, pointer :: f
f => ptr
end function f
end
Indeed that I meant.
This works as expected with Intel and AOCC, but gives a
syntax error with every gfortran tested because of match.c:
alloc_opt_list:
m = gfc_match (" stat = %v", &tmp);
I think we can simply change that one to %e; the definable
check should ensure that any non variable (in the Fortran sense)
is rejected.
And we should update the comment for %v / match_variable to state
that it does not include function references.
In some cases, like with OpenMP, we still do not want to match
functions, hence, changing match_variable is probably not what we
want to do. Additionally, for all %v replaced by %e we need to
ensure that there is a definable check. (Which should be there
already as INTENT(IN) or named constants or ... are also invalid.)
Also affected: Some I/O items, a bunch of other stat=%v and
errmsg=%v.
Talking about errmsg: In the same function, the same check is
done for errmsg as for stat – hence, the patch should update
also errmsg.
Additionally, I have to admit that I do not understand the
following existing condition, which you did not touch:
if ((stat->ts.type != BT_INTEGER
&& !(stat->ref && (stat->ref->type == REF_ARRAY
|| stat->ref->type == REF_COMPONENT)))
|| stat->rank > 0)
gfc_error ("Stat-variable at %L must be a scalar INTEGER "
"variable", &stat->where);
I mean the ts.type != BT_INTEGER and stat->rank != 0 is clear,
but what's the reason for the refs?
Well, that needs to be answered by Steve (see commit 3759634).
(https://gcc.gnu.org/g:3759634f3208cbc1226bec19d22cbff989a287c3 (svn
r145331))
The reason for the ref checks is unclear and seem to be wrong. The added
testcases also only use 'x' (real) and n or i (integer) as input, i.e.
they do not exercise this. I did not look for the patch email for reasoning.
Thanks,
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Re: [PATCH 3/3] [PR libfortran/101305] Fix ISO_Fortran_binding.h paths in gfortran testsuite
Hi Sandra,
On 21.07.21 12:17, Tobias Burnus wrote:
On 13.07.21 23:28, Sandra Loosemore wrote:
ISO_Fortran_binding.h is now generated in the libgfortran build
directory where it is on the default include path. Adjust includes in
the gfortran testsuite not to include an explicit path pointing at the
source directory.
...
-#include "../../../libgfortran/ISO_Fortran_binding.h"
+#include "ISO_Fortran_binding.h"
Unfortunately, that does not help.
It seems as if the following works in the *.exp file:
# Flags for finding libgfortran ISO*.h files.
if [info exists TOOL_OPTIONS] {
set specpath [get_multilibs ${TOOL_OPTIONS}]
} else {
set specpath [get_multilibs]
}
set options "-I $specpath/libgfortran/"
I am not sure whether that should/can be added into
gfortran.dg/dg.exp
or whether we only want to do this in
ts29113/ts29113.exp
alias
c-interop/interop.exp
f18-c-interop/interop.exp
...
That seems to work fine with -m32 and -m64.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Re: [PATCH] PR fortran/101536 - ICE in gfc_conv_expr_descriptor, at fortran/trans-array.c:7324
Hi Tobias,
> > However, an additional plain check on e->rank != 0 also in the
> > CLASS cases fixes the original issue as well as your example:
> [...]
> > And regtests ok. :-)
> > See attached updated patch.
>
> I think you still need to remove the 'return true;' from
> the 'if (e->rank != 0 && e->ts.type == BT_CLASS' block – to
> fall through to the e->rank check after the block.
> (When 'return true;' is gone, the '{' and '}' can also be removed.)
>
> Reason: Assume 'CLASS(...) x'. In this case, 'x' is a scalar.
> And even after calling gfc_add_class_array_ref it remains
> a scalar and e->rank == 0.
>
> Or in other words: I think with your current patch,
> class(u) :: z
> f = size (z)
> is wrongly accepted without an error.
did you really check that? My related testing succeeded without
and with the return (which was in the original commit by Paul).
I have nevertheless followed your advice to remove the return
statement, extended the testcase and regtested again.
Committed as https://gcc.gnu.org/g:e314cfc371d8b2405a1d81e51b90f9fb24b9061f
Thanks,
Harald
Re: [PATCH 3/3] [PR libfortran/101305] Fix ISO_Fortran_binding.h paths in gfortran testsuite
On 7/23/21 8:15 AM, Tobias Burnus wrote:
Hi Sandra,
On 21.07.21 12:17, Tobias Burnus wrote:
On 13.07.21 23:28, Sandra Loosemore wrote:
ISO_Fortran_binding.h is now generated in the libgfortran build
directory where it is on the default include path. Adjust includes in
the gfortran testsuite not to include an explicit path pointing at the
source directory.
...
-#include "../../../libgfortran/ISO_Fortran_binding.h"
+#include "ISO_Fortran_binding.h"
Unfortunately, that does not help.
It seems as if the following works in the *.exp file:
# Flags for finding libgfortran ISO*.h files.
if [info exists TOOL_OPTIONS] {
set specpath [get_multilibs ${TOOL_OPTIONS}]
} else {
set specpath [get_multilibs]
}
set options "-I $specpath/libgfortran/"
I am not sure whether that should/can be added into
gfortran.dg/dg.exp
or whether we only want to do this in
ts29113/ts29113.exp
alias
c-interop/interop.exp
f18-c-interop/interop.exp
...
That seems to work fine with -m32 and -m64.
Well, given that the original patch in this thread was for tests outside
the ts29113 testsuite, any fix has to go someplace where those tests
would pick it up too.
I'm not seeing the include path failures Tobias is seeing, so I can't
confirm his change fixes them, either. When I do "make check-fortran"
in my build tree, it seems to be finding the include files because of
there being a pile of -B options added to the gfortran command line. I
don't know where those are coming from, or why this isn't working for
Tobias. :-S
-Sandra
