Re: [patch, Fortran] Fix PR 119157, regression with -Wexternal-argument-mismatch

[Thomas Koenig]

Am 09.03.25 um 17:54 schrieb Mikael Morin:

(Actually, none of the cases you mentioned above can happen, at least
not now, but you're right).  Patch which does this approved?


Sure.


Committed after regression-testing.

Thanks for the hint (and the pre-review :-)

Best regards

Thomas

commit 9f5b508bc5c16ae11ea385f6031487a518f62c8f (HEAD -> master)
Author: Thomas Koenig 
Date:   Sun Mar 9 19:35:06 2025 +0100

Use gfc_commit_symbol() to remove UNDO status instead of new function.

This is a cleaner version, removing an unneeded function and
making sure that no memory leaks can occur if callers change.

gcc/fortran/ChangeLog:

PR fortran/119157
* gfortran.h (gfc_pop_undo_symbol): Remove prototype.
* interface.cc (gfc_get_formal_from_actual_arglist): Use
gfc_commit_symbol() instead of gfc_pop_undo_symbo
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index f81be1d984c..cf48d025768 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3736,7 +3736,6 @@ void gfc_traverse_user_op (gfc_namespace *, void 
(*)(gfc_user_op *));
 void gfc_save_all (gfc_namespace *);
 
 void gfc_enforce_clean_symbol_state (void);
-void gfc_pop_undo_symbol (void);
 
 gfc_gsymbol *gfc_get_gsymbol (const char *, bool bind_c);
 gfc_gsymbol *gfc_find_gsymbol (gfc_gsymbol *, const char *);
diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index e3bc22f25e5..c59ed1f5306 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -5836,8 +5836,6 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sym,
{
  snprintf (name, GFC_MAX_SYMBOL_LEN, "_formal_%d", var_num ++);
  gfc_get_symbol (name, gfc_current_ns, &s);
- /* We do not need this in an undo table.  */
- gfc_pop_undo_symbol();
  if (a->expr->ts.type == BT_PROCEDURE)
{
  gfc_symbol *asym = a->expr->symtree->n.sym;
@@ -5878,12 +5876,14 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sym,
  s->declared_at = a->expr->where;
  s->attr.intent = INTENT_UNKNOWN;
  (*f)->sym = s;
+ gfc_commit_symbol (s);
}
   else  /* If a->expr is NULL, this is an alternate rerturn.  */
(*f)->sym = NULL;
 
   f = &((*f)->next);
 }
+
 }
 
 
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 92cba418784..81aa81df2ee 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3898,11 +3898,6 @@ enforce_single_undo_checkpoint (void)
   gcc_checking_assert (single_undo_checkpoint_p ());
 }
 
-void
-gfc_pop_undo_symbol ()
-{
-  latest_undo_chgset->syms.pop();
-}
 
 /* Undoes all the changes made to symbols in the current statement.  */
 

Re: [patch, Fortran] Fix PR 119157, regression with -Wexternal-argument-mismatch

[Thomas Koenig]

Hi Mikael,


_symbol *asym = a->expr->symtree->n.sym;


You may consider calling gfc_commit_symbol(s) instead at the end of the 
function body.  It might avoid leaking the old_symbol field in case the 
function is called more than once or the symbol is already existing (not 
completely clear whether that can happen).


You're right, this is the cleaner solution.

(Actually, none of the cases you mentioned above can happen, at least
not now, but you're right).  Patch which does this approved?

Best regards

Thomas

Re: [patch, Fortran] Fix PR 119157, regression with -Wexternal-argument-mismatch

[Mikael Morin]

Le 09/03/2025 à 17:40, Thomas Koenig a écrit :

Hi Mikael,


_symbol *asym = a->expr->symtree->n.sym;


You may consider calling gfc_commit_symbol(s) instead at the end of 
the function body.  It might avoid leaking the old_symbol field in 
case the function is called more than once or the symbol is already 
existing (not completely clear whether that can happen).


You're right, this is the cleaner solution.

(Actually, none of the cases you mentioned above can happen, at least
not now, but you're right).  Patch which does this approved?


Sure.

Re: [patch, Fortran] Fix PR 119157, regression with -Wexternal-argument-mismatch

[Mikael Morin]

Hello,

sorry to come late to the party.

Le 08/03/2025 à 13:52, Thomas Koenig a écrit :


diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index edec907d33a..e3bc22f25e5 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -5836,6 +5836,8 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sym,
{
  snprintf (name, GFC_MAX_SYMBOL_LEN, "_formal_%d", var_num ++);
  gfc_get_symbol (name, gfc_current_ns, &s);
+ /* We do not need this in an undo table.  */
+ gfc_pop_undo_symbol();
  if (a->expr->ts.type == BT_PROCEDURE)
{
  gfc_symbol *asym = a->expr->symtree->n.sym;


You may consider calling gfc_commit_symbol(s) instead at the end of the 
function body.  It might avoid leaking the old_symbol field in case the 
function is called more than once or the symbol is already existing (not 
completely clear whether that can happen).


Mikael