diff mbox series

[fortran] PRs 105152, 100193, 87946, 103389, 104429 and 82774

Message ID CAGkQGiLQB=optugqew-T1a5bn=DA=XsN7fYT=hT4fY4UtpU7+Q@mail.gmail.com
State New
Headers show
Series [fortran] PRs 105152, 100193, 87946, 103389, 104429 and 82774 | expand

Commit Message

Paul Richard Thomas April 22, 2023, 8:32 a.m. UTC
Hi All,

As usual, I received a string of emails on retargeting for PRs for which I
was either responsible or was on the cc list. This time I decided to take a
look at them all, in order to reward the tireless efforts of Richi, Jakub
and Martin with some attention at least.

I have fixed the PRs in the title line: See the attached changelog, patch
and testcases.

OK for 14-branch?

Of the others:
PR100815 - fixed already for 12-branch on. Martin located the fix from
Tobias, for which thanks. It's quite large but has stood the test of time.
Should I backport to 11-branch?
PR103366 - fixed on 12-branch on. I closed it.
PR103715 - might be fixed but the report is for gcc with checking enabled.
I will give that a go.
PR103716 - a gimple problem with assumed shape characters. A TODO.
PR103931 - I couldn't reproduce the bug, which involves 'ambiguous c_ptr'.
To judge by the comments, it seems that this bug is a bit elusive.
PR65381 - Seems to be fixed for 12-branch on
PR82064 - Seems to be fixed.
PR83209 - Coarray allocation - seems to be fixed.
PR84244 - Coarray segfault. I have no acquaintance with the inner works of
coarrays and so don't think that I can fix this one.
PR87674 - Segfault in runtime with non-overridable proc-pointer. A TODO.
PR96087 - A module procedure problem. A TODO.

I have dejagnu-ified testcases for the already fixed PRs ready to go.
Should these be committed or do we assume that the fixes already provided
adequate tests?

Regards

Paul

Comments

Harald Anlauf April 23, 2023, 9:48 p.m. UTC | #1
Hi Paul,

Am 22.04.23 um 10:32 schrieb Paul Richard Thomas via Gcc-patches:
> Hi All,
>
> As usual, I received a string of emails on retargeting for PRs for which I
> was either responsible or was on the cc list. This time I decided to take a
> look at them all, in order to reward the tireless efforts of Richi, Jakub
> and Martin with some attention at least.
>
> I have fixed the PRs in the title line: See the attached changelog, patch
> and testcases.
>
> OK for 14-branch?

the patch looks essentially good to me.

Can you please have a look at testcase pr100193.f90, which fails
for me because the module file is not generated and there is no
corresponding dg-pattern:

FAIL: gfortran.dg/pr100193.f90   -O  (test for excess errors)

                 === gfortran Summary ===

# of expected passes            1
# of unexpected failures        1

You could either simply omit the main program or add a pattern.
(The shortened testcase would still fail w/o the patch.)

> Of the others:
> PR100815 - fixed already for 12-branch on. Martin located the fix from
> Tobias, for which thanks. It's quite large but has stood the test of time.
> Should I backport to 11-branch?
> PR103366 - fixed on 12-branch on. I closed it.
> PR103715 - might be fixed but the report is for gcc with checking enabled.
> I will give that a go.
> PR103716 - a gimple problem with assumed shape characters. A TODO.
> PR103931 - I couldn't reproduce the bug, which involves 'ambiguous c_ptr'.
> To judge by the comments, it seems that this bug is a bit elusive.
> PR65381 - Seems to be fixed for 12-branch on
> PR82064 - Seems to be fixed.
> PR83209 - Coarray allocation - seems to be fixed.
> PR84244 - Coarray segfault. I have no acquaintance with the inner works of
> coarrays and so don't think that I can fix this one.
> PR87674 - Segfault in runtime with non-overridable proc-pointer. A TODO.
> PR96087 - A module procedure problem. A TODO.
>
> I have dejagnu-ified testcases for the already fixed PRs ready to go.
> Should these be committed or do we assume that the fixes already provided
> adequate tests?

I think this depends.  A testcase that is "sufficiently orthogonal" to
those in the testsuite may be worth to be added.  Otherwise randomly
adding testcases might just increase the runtime for regression testing,
which could be counter-productive for the development process.  So
better really decide on a case-by-case basis?

(Of course this is only my opinion, and other may have a different view
upon this.)

I checked PR100815.  The testcase in comment#0 appears to work for me
for all open branches (10 to 14).  The commit that supposedly fixed
the issue applies to 12-branch and newer.  Either there is something
else in 11-branch which fixed it in a different way, or the bisecting
unfortunately pointed to the wrong commit.  And since there is no
traceback information in the PR, I am simply confused.
So do you think this testcase improves coverage and thus adds value?

PR103715: with valgrind I get invalid reads, so I guess there is
something lurking here and it only appears to be fixed.

PR103931: it is indeed a bit elusive, but very sensitive to code
changes.  Also Bernhard had a look at it.  Given that there are
a couple of bugs related to module reading, and rename-on-use,
I'd recommend to leave that open for further analysis.

PR65381: works for me even on 11-branch.  I think this looks very
much like a duplicate of a PR that was fixed by Tobias.  Still
fails on 10-branch, but might not be worth fixing there.  Simply
close it as 10-only?

PR103716: I think that one is interesting, as there are a couple
of PRs involving inquiry functions.

> Regards
>
> Paul

Cheers,
Harald
Bernhard Reutner-Fischer April 24, 2023, 4:41 p.m. UTC | #2
On Sun, 23 Apr 2023 23:48:03 +0200
Harald Anlauf via Fortran <fortran@gcc.gnu.org> wrote:

> Am 22.04.23 um 10:32 schrieb Paul Richard Thomas via Gcc-patches:

> > PR103931 - I couldn't reproduce the bug, which involves 'ambiguous c_ptr'.
> > To judge by the comments, it seems that this bug is a bit elusive.

See Haralds comment 12, you need to remove the use cmodule:
module DModule
   use AModule
   !comment 12, 'use CModule' should not be needed: use CModule
   !use CModule

   implicit none
   private

   public :: DType

   type, abstract :: DType
   end type
end module

> PR103931: it is indeed a bit elusive, but very sensitive to code
> changes.  Also Bernhard had a look at it.  Given that there are
> a couple of bugs related to module reading, and rename-on-use,
> I'd recommend to leave that open for further analysis.

I would mark the dt sym that is used *as* the generic interface with
attr.generic.

Like: https://gcc.gnu.org/PR103931#c18

This seems to work and sounds somewhat plausible (to me).
If that is not correct, then i'm running out of ideas and will stop
looking at that PR.

cheers,
diff mbox series

Patch

diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index e9843e9549c..fa505ab7ed9 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -3312,6 +3312,16 @@  gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
 	    }
 	}
 
+      if (UNLIMITED_POLY (a->expr)
+	  && !(f->sym->ts.type == BT_ASSUMED || UNLIMITED_POLY (f->sym)))
+	{
+	  gfc_error ("Unlimited polymorphic actual argument at %L is not "
+		     "matched with either an unlimited polymorphic or "
+		     "assumed type dummy argument", &a->expr->where);
+	  ok = false;
+	  goto match;
+	}
+
       /* Special case for character arguments.  For allocatable, pointer
 	 and assumed-shape dummies, the string length needs to match
 	 exactly.  */
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 55d8e326a87..aaca772320a 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -11129,6 +11129,17 @@  resolve_ordinary_assign (gfc_code *code, gfc_namespace *ns)
   lhs = code->expr1;
   rhs = code->expr2;
 
+  if ((lhs->symtree->n.sym->ts.type == BT_DERIVED
+       || lhs->symtree->n.sym->ts.type == BT_CLASS)
+      && !lhs->symtree->n.sym->attr.proc_pointer
+      && gfc_expr_attr (lhs).proc_pointer)
+    {
+      gfc_error ("Variable in the ordinary assignment at %L is a procedure "
+		 "pointer component",
+		 &lhs->where);
+      return false;
+    }
+
   if ((gfc_numeric_ts (&lhs->ts) || lhs->ts.type == BT_LOGICAL)
       && rhs->ts.type == BT_CHARACTER
       && (rhs->expr_type != EXPR_CONSTANT || !flag_dec_char_conversions))
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e1725808033..6c47b537dfc 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -11471,6 +11471,12 @@  gfc_walk_array_ref (gfc_ss * ss, gfc_expr * expr, gfc_ref * ref)
 	  break;
 
 	case AR_FULL:
+	  /* Assumed shape arrays from interface mapping need this fix.  */
+	  if (!ar->as && expr->symtree->n.sym->as)
+	    {
+	      ar->as = gfc_get_array_spec();
+	      *ar->as = *expr->symtree->n.sym->as;
+	    }
 	  newss = gfc_get_array_ss (ss, expr, ar->as->rank, GFC_SS_SECTION);
 	  newss->info->data.array.ref = ref;
 
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 09cdd9263c4..74d6948b0ae 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -996,6 +996,12 @@  gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e,
   tree var;
   tree tmp;
   int dim;
+  bool unlimited_poly;
+
+  unlimited_poly = class_ts.type == BT_CLASS
+		   && class_ts.u.derived->components->ts.type == BT_DERIVED
+		   && class_ts.u.derived->components->ts.u.derived
+						->attr.unlimited_polymorphic;
 
   /* The intrinsic type needs to be converted to a temporary
      CLASS object.  */
@@ -1067,9 +1073,7 @@  gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e,
     }
 
   gcc_assert (class_ts.type == BT_CLASS);
-  if (class_ts.u.derived->components->ts.type == BT_DERIVED
-      && class_ts.u.derived->components->ts.u.derived
-		 ->attr.unlimited_polymorphic)
+  if (unlimited_poly)
     {
       ctree = gfc_class_len_get (var);
       /* When the actual arg is a char array, then set the _len component of the
@@ -1116,10 +1120,7 @@  gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e,
 
       gfc_add_modify (&parmse->pre, ctree, fold_convert (TREE_TYPE (ctree), tmp));
     }
-  else if (class_ts.type == BT_CLASS
-	   && class_ts.u.derived->components
-	   && class_ts.u.derived->components->ts.u
-		.derived->attr.unlimited_polymorphic)
+  else if (unlimited_poly)
     {
       ctree = gfc_class_len_get (var);
       gfc_add_modify (&parmse->pre, ctree,
@@ -5650,7 +5651,7 @@  gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
 	  itype = CFI_type_other;  // FIXME: Or CFI_type_cptr ?
 	  break;
 	case BT_CLASS:
-	  if (UNLIMITED_POLY (e) && fsym->ts.type == BT_ASSUMED)
+	  if (fsym->ts.type == BT_ASSUMED)
 	    {
 	      // F2017: 7.3.2.2: "An entity that is declared using the TYPE(*)
 	      // type specifier is assumed-type and is an unlimited polymorphic
@@ -6682,20 +6683,11 @@  gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			{
 			  tree zero;
 
-			  gfc_expr *var;
-
-			  /* Borrow the function symbol to make a call to
-			     gfc_add_finalizer_call and then restore it.  */
-			  tmp = e->symtree->n.sym->backend_decl;
-			  e->symtree->n.sym->backend_decl
-					= TREE_OPERAND (parmse.expr, 0);
-			  e->symtree->n.sym->attr.flavor = FL_VARIABLE;
-			  var = gfc_lval_expr_from_sym (e->symtree->n.sym);
-			  finalized = gfc_add_finalizer_call (&parmse.post,
-							      var);
-			  gfc_free_expr (var);
-			  e->symtree->n.sym->backend_decl = tmp;
-			  e->symtree->n.sym->attr.flavor = FL_PROCEDURE;
+			  /* Finalize the expression.  */
+			  gfc_finalize_tree_expr (&parmse, NULL,
+						  gfc_expr_attr (e), e->rank);
+			  gfc_add_block_to_block (&parmse.post,
+						  &parmse.finalblock);
 
 			  /* Then free the class _data.  */
 			  zero = build_int_cst (TREE_TYPE (parmse.expr), 0);
@@ -7131,7 +7123,15 @@  gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 		 types passed to class formals need the _data component.  */
 	      tmp = gfc_class_data_get (tmp);
 	      if (!CLASS_DATA (fsym)->attr.dimension)
-		tmp = build_fold_indirect_ref_loc (input_location, tmp);
+		{
+		  if (UNLIMITED_POLY (fsym))
+		    {
+		      tree type = gfc_typenode_for_spec (&e->ts);
+		      type = build_pointer_type (type);
+		      tmp = fold_convert (type, tmp);
+		    }
+		  tmp = build_fold_indirect_ref_loc (input_location, tmp);
+		}
 	    }
 
 	  if (e->expr_type == EXPR_OP
@@ -8767,11 +8767,9 @@  gfc_trans_alloc_subarray_assign (tree dest, gfc_component * cm,
 /* Allocate or reallocate scalar component, as necessary.  */
 
 static void
-alloc_scalar_allocatable_for_subcomponent_assignment (stmtblock_t *block,
-						      tree comp,
-						      gfc_component *cm,
-						      gfc_expr *expr2,
-						      gfc_symbol *sym)
+alloc_scalar_allocatable_subcomponent (stmtblock_t *block, tree comp,
+				       gfc_component *cm, gfc_expr *expr2,
+				       tree slen)
 {
   tree tmp;
   tree ptr;
@@ -8789,26 +8787,20 @@  alloc_scalar_allocatable_for_subcomponent_assignment (stmtblock_t *block,
 
   if (cm->ts.type == BT_CHARACTER && cm->ts.deferred)
     {
-      char name[GFC_MAX_SYMBOL_LEN+9];
-      gfc_component *strlen;
-      /* Use the rhs string length and the lhs element size.  */
       gcc_assert (expr2->ts.type == BT_CHARACTER);
-      if (!expr2->ts.u.cl->backend_decl)
-	{
-	  gfc_conv_string_length (expr2->ts.u.cl, expr2, block);
-	  gcc_assert (expr2->ts.u.cl->backend_decl);
-	}
+      if (!expr2->ts.u.cl->backend_decl
+	  || !VAR_P (expr2->ts.u.cl->backend_decl))
+	expr2->ts.u.cl->backend_decl = gfc_create_var (TREE_TYPE (slen),
+						       "slen");
+      gfc_add_modify (block, expr2->ts.u.cl->backend_decl, slen);
 
       size = expr2->ts.u.cl->backend_decl;
 
-      /* Ensure that cm->ts.u.cl->backend_decl is a componentref to _%s_length
-	 component.  */
-      sprintf (name, "_%s_length", cm->name);
-      strlen = gfc_find_component (sym, name, true, true, NULL);
+      gfc_deferred_strlen (cm, &tmp);
       lhs_cl_size = fold_build3_loc (input_location, COMPONENT_REF,
 				     gfc_charlen_type_node,
 				     TREE_OPERAND (comp, 0),
-				     strlen->backend_decl, NULL_TREE);
+				     tmp, NULL_TREE);
 
       tmp = TREE_TYPE (gfc_typenode_for_spec (&cm->ts));
       tmp = TYPE_SIZE_UNIT (tmp);
@@ -8881,8 +8873,8 @@  alloc_scalar_allocatable_for_subcomponent_assignment (stmtblock_t *block,
 /* Assign a single component of a derived type constructor.  */
 
 static tree
-gfc_trans_subcomponent_assign (tree dest, gfc_component * cm, gfc_expr * expr,
-			       gfc_symbol *sym, bool init)
+gfc_trans_subcomponent_assign (tree dest, gfc_component * cm,
+			       gfc_expr * expr, bool init)
 {
   gfc_se se;
   gfc_se lse;
@@ -8976,19 +8968,17 @@  gfc_trans_subcomponent_assign (tree dest, gfc_component * cm, gfc_expr * expr,
 	   || (cm->ts.type == BT_CLASS && CLASS_DATA (cm)->attr.allocatable
 	       && expr->ts.type != BT_CLASS)))
     {
+      gfc_init_se (&se, NULL);
+      gfc_conv_expr (&se, expr);
+
       /* Take care about non-array allocatable components here.  The alloc_*
 	 routine below is motivated by the alloc_scalar_allocatable_for_
 	 assignment() routine, but with the realloc portions removed and
 	 different input.  */
-      alloc_scalar_allocatable_for_subcomponent_assignment (&block,
-							    dest,
-							    cm,
-							    expr,
-							    sym);
+      alloc_scalar_allocatable_subcomponent (&block, dest, cm, expr,
+					     se.string_length);
       /* The remainder of these instructions follow the if (cm->attr.pointer)
 	 if (!cm->attr.dimension) part above.  */
-      gfc_init_se (&se, NULL);
-      gfc_conv_expr (&se, expr);
       gfc_add_block_to_block (&block, &se.pre);
 
       if (expr->symtree && expr->symtree->n.sym->attr.proc_pointer
@@ -9252,13 +9242,11 @@  gfc_trans_structure_assign (tree dest, gfc_expr * expr, bool init, bool coarray)
       if (!c->expr)
 	{
 	  gfc_expr *e = gfc_get_null_expr (NULL);
-	  tmp = gfc_trans_subcomponent_assign (tmp, cm, e, expr->ts.u.derived,
-					       init);
+	  tmp = gfc_trans_subcomponent_assign (tmp, cm, e, init);
 	  gfc_free_expr (e);
 	}
       else
-        tmp = gfc_trans_subcomponent_assign (tmp, cm, c->expr,
-                                             expr->ts.u.derived, init);
+        tmp = gfc_trans_subcomponent_assign (tmp, cm, c->expr, init);
       gfc_add_expr_to_block (&block, tmp);
     }
   return gfc_finish_block (&block);