[Patch, fortran] PR84674(12-15 regression) and PR117730 - non_overridable typebound proc problems
Hi All, I was going through some of the older regressions and found pr84674, which was distinctly low hanging fruit because the contributor has found the offending bit of code. However, buried in this PR, on the grounds that it looked similar, was what has now become pr117730. This was quite difficult to diagnose and the cause of the problem was only found by instrumenting class.c (add_proc_comp) to check the order in which non_overridable procedure components were added to the vtables of derived type extensions that are in different modules to the parent. Failure to keep the order the same results, of course, in the wrong procedure being called. The fixes for these PRs verge on 'obvious' but I thought that I should submit them to the list because I want to push and backport them together. Although PR117730 is not a regression, I think that it is sufficiently limiting that it should be backported to active branches. OK for mainline and, after a week or two, backporting to 13- and 14-branches/ Paul Fortran: Fix non_overridable typebound proc problems [PR84674/117730]. 2024-11-23 Paul Thomas gcc/fortran/ChangeLog PR fortran/117730 * class.c (add_proc_comp): Only reject a non_overridable if it has no overridden procedure and the component is already present in the vtype. PR fortran/84674 * resolve.cc (resolve_fl_derived): Do not build a vtable for a derived type extension that is completely empty. gcc/testsuite/ChangeLog PR fortran/117730 * gfortran.dg/pr117730_a.f90: New test. * gfortran.dg/pr117730_b.f90: New test. PR fortran/84674 * gfortran.dg/pr84674.f90: New test. diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc index fc709fec322..388891a2fd5 100644 --- a/gcc/fortran/class.cc +++ b/gcc/fortran/class.cc @@ -886,11 +886,12 @@ add_proc_comp (gfc_symbol *vtype, const char *name, gfc_typebound_proc *tb) { gfc_component *c; - if (tb->non_overridable && !tb->overridden) -return; c = gfc_find_component (vtype, name, true, true, NULL); + if (tb->non_overridable && !tb->overridden && c) +return; + if (c == NULL) { /* Add procedure component. */ diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index e8f780d1ef9..79f73e7ec40 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -16288,6 +16288,10 @@ resolve_fl_derived (gfc_symbol *sym) && sym->ns->proc_name && sym->ns->proc_name->attr.flavor == FL_MODULE && sym->attr.access != ACCESS_PRIVATE + && !(sym->attr.extension + && sym->attr.zero_comp + && !sym->f2k_derived->tb_sym_root + && !sym->f2k_derived->tb_uop_root) && !(sym->attr.vtype || sym->attr.pdt_template)) { gfc_symbol *vtab = gfc_find_derived_vtab (sym); diff --git a/gcc/testsuite/gfortran.dg/pr117730_a.f90 b/gcc/testsuite/gfortran.dg/pr117730_a.f90 new file mode 100644 index 000..12e28214b02 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr117730_a.f90 @@ -0,0 +1,50 @@ +! { dg-do compile } +! +! Test the fix for PR117730 in which the non_overrridable procedures in 'child' +! were mixied up in the vtable for the extension 'child2' in pr117730_b.f90. +! This resulted in 'this%calc()' in 'function child_get(this)' returning garbage +! when 'this' was of dynamic type 'child2'. +! +! Contributed by in comment 4 of PR84674. +! +module module1 +implicit none +private +public :: child + +type, abstract :: parent +contains +procedure, pass :: reset => parent_reset +end type parent + +type, extends(parent), abstract :: child +contains +procedure, pass, non_overridable :: reset => child_reset +procedure, pass, non_overridable :: get => child_get +procedure(calc_i), pass, deferred :: calc +end type child + +abstract interface +pure function calc_i(this) result(value) +import :: child +class(child), intent(in) :: this +integer :: value +end function calc_i +end interface + +contains +pure subroutine parent_reset(this) +class(parent), intent(inout) :: this +end subroutine parent_reset + +pure subroutine child_reset(this) +class(child), intent(inout) :: this +end subroutine child_reset + +function child_get(this) result(value) +class(child), intent(inout) :: this +integer :: value + +value = this%calc() +end function child_get +end module module1 diff --git a/gcc/testsuite/gfortran.dg/pr117730_b.f90 b/gcc/testsuite/gfortran.dg/pr117730_b.f90 new file mode 100644 index 000..09707882989 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr117730_b.f90 @@ -0,0 +1,47 @@ +! { dg-do run } +! { dg-compile-aux-modules "pr117730_a.f90" } +! { dg-additional-sources pr117730_a.f90 } +! +! Test the fix for PR117730 in which the non_overrridable procedures in +! pr117730_a.f90 were mixied up in the vtable for 'child2' below. This resulted +! in 'this%calc()' in 'fun
Re: [Patch, fortran] PR84674(12-15 regression) and PR117730 - non_overridable typebound proc problems
Hi Paul, Am 23.11.24 um 17:05 schrieb Paul Richard Thomas: Hi All, I was going through some of the older regressions and found pr84674, which was distinctly low hanging fruit because the contributor has found the offending bit of code. However, buried in this PR, on the grounds that it looked similar, was what has now become pr117730. This was quite difficult to diagnose and the cause of the problem was only found by instrumenting class.c (add_proc_comp) to check the order in which non_overridable procedure components were added to the vtables of derived type extensions that are in different modules to the parent. Failure to keep the order the same results, of course, in the wrong procedure being called. The fixes for these PRs verge on 'obvious' but I thought that I should submit them to the list because I want to push and backport them together. Although PR117730 is not a regression, I think that it is sufficiently limiting that it should be backported to active branches. they are indeed almost obvious and fix possibly annoying wrong-code issues. OK for mainline and, after a week or two, backporting to 13- and 14-branches/ OK for mainline and backports. Thanks for the patch! Harald Paul Fortran: Fix non_overridable typebound proc problems [PR84674/117730]. 2024-11-23 Paul Thomas gcc/fortran/ChangeLog PR fortran/117730 * class.c (add_proc_comp): Only reject a non_overridable if it has no overridden procedure and the component is already present in the vtype. PR fortran/84674 * resolve.cc (resolve_fl_derived): Do not build a vtable for a derived type extension that is completely empty. gcc/testsuite/ChangeLog PR fortran/117730 * gfortran.dg/pr117730_a.f90: New test. * gfortran.dg/pr117730_b.f90: New test. PR fortran/84674 * gfortran.dg/pr84674.f90: New test.
Re: [wwwdocs] Fortran changes, add description regarding commas in formats
On 11/23/24 10:59 AM, Harald Anlauf wrote: Am 23.11.24 um 19:35 schrieb Jerry D: Suggested docs change regarding fix to PR88052. See attached diff file. OK to push? Regards, Jerry Jerry, for clarification: isn't it the language standard option used when compiling the main that is relevant? The main might be compiled separately from a module containing the offending legacy code, where it may or may not be diagnosed at compile time. Otherwise OK from my side. Thanks, Harald Indeed, only the -std= specified on the main program unit is relevant. I will clarify this in my description. I am working on a separate patch that will check the format strings at compile time when possible. We do not check format strings in character variables at compile time and I am not planning to this. (It might be possible in a translation or resolve stage if the character variable is a constant). I will update and push this doc change. Thanks Herald.
Re: [libgfortran, patch] PR88052 Format contravening constraint C1002 permitted
Hi Jerry, I had originally created this patch in 2018 and we did not get back to it. This results in more restrictive runtime behavior. I will go through the front-end code with another patch to catch this at compile time. Changelog and new test case. See attached patch. OK for trunk Yes, OK. Can you also make a mention of this in https://gcc.gnu.org/gcc-15/changes.html ? Best regards Thomas
Re: [libgfortran, patch] PR88052 Format contravening constraint C1002 permitted
On 11/23/24 12:40 AM, Thomas Koenig wrote: Hi Jerry, I had originally created this patch in 2018 and we did not get back to it. This results in more restrictive runtime behavior. I will go through the front-end code with another patch to catch this at compile time. Changelog and new test case. See attached patch. OK for trunk Yes, OK. Can you also make a mention of this in https://gcc.gnu.org/gcc-15/changes.html ? Best regards Thomas Thanks for review, pushed. Working on the docs change now. Jerry
Re: [wwwdocs] Fortran changes, add description regarding commas in formats
Am 23.11.24 um 19:35 schrieb Jerry D: Suggested docs change regarding fix to PR88052. See attached diff file. OK to push? Regards, Jerry Jerry, for clarification: isn't it the language standard option used when compiling the main that is relevant? The main might be compiled separately from a module containing the offending legacy code, where it may or may not be diagnosed at compile time. Otherwise OK from my side. Thanks, Harald
[wwwdocs] Fortran changes, add description regarding commas in formats
Suggested docs change regarding fix to PR88052. See attached diff file. OK to push? Regards, Jerry diff --git a/htdocs/gcc-15/changes.html b/htdocs/gcc-15/changes.html index 46dad391..2dc222e3 100644 --- a/htdocs/gcc-15/changes.html +++ b/htdocs/gcc-15/changes.html @@ -143,6 +143,10 @@ a work-in-progress. >gfortran documentation for details. These have been proposed (https://j3-fortran.org/doc/year/24/24-116.txt";>J3/24-116) for inclusion in the next Fortran standard. + Missing commas separating descriptors in input/output format strings are no + longer permitted by default and are rejected at run-time unless -std=legacy + is used at compile time. See Fortran 2023 constraint C1302. +