Re: [Patch, fortran] PR103312 - [11/12/13/14/15 Regression] ICE in gfc_find_component since r9-1098-g3cf89a7b992d483e
Hi Harald,
You were absolutely right about returning 'false' :-) The patch is duly
corrected.
Committed to mainline and will be followed by backports in a few weeks.
Regards
Paul
On Tue, 21 May 2024 at 19:58, Harald Anlauf wrote:
> Hi Paul,
>
> Am 20.05.24 um 11:06 schrieb Paul Richard Thomas:
> > Hi All,
> >
> > I don't think that this PR is really a regression although the fact that
> it
> > is marked as such brought it to my attention :-)
> >
> > The fix turned out to be remarkably simple. It was found after going
> down a
> > silly number of rabbit holes, though!
> >
> > The chunk in dependency.cc is probably more elaborate than it needs to
> be.
> > Returning -2 is sufficient for the testcase to work. Otherwise, the
> > comments in the patch say it all.
>
> this part looks OK, but can you elaborate on this change to expr.cc:
>
> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
> index c883966646c..4ee2ad55915 100644
> --- a/gcc/fortran/expr.cc
> +++ b/gcc/fortran/expr.cc
> @@ -3210,6 +3210,11 @@ gfc_reduce_init_expr (gfc_expr *expr)
> {
> bool t;
>
> + /* It is far too early to resolve a class compcall. Punt to
> resolution. */
> + if (expr && expr->expr_type == EXPR_COMPCALL
> + && expr->symtree->n.sym->ts.type == BT_CLASS)
> +return true;
> +
>
> I would have expected to return 'false' here, as we do not
> have an expression that reduces to a constant. What am I
> missing?
>
> (The testcase compiles and works here also when using 'false'.)
>
> > OK for mainline? I will delay for a month before backporting.
>
> OK if can you show me wrong...
>
> Thanks,
> Harald
>
> > Regards
> >
> > Paul
> >
>
>
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 13/05/2024 à 09:25, Mikael Morin a écrit : Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. Hello, I'm coming back to this. I think either one of Steve's patch or your variant in the PR is a better fix for the ICE as a first step; they seem less fragile at least. Then we can look at a possible reordering of conflict checks as with the patch you originally submitted in this thread. Mikael
[PATCH, v2] Fortran: improve attribute conflict checking [PR93635]
Hi Mikael,
On 5/23/24 09:49, Mikael Morin wrote:
Le 13/05/2024 à 09:25, Mikael Morin a écrit :
Le 10/05/2024 à 21:56, Harald Anlauf a écrit :
Am 10.05.24 um 21:48 schrieb Harald Anlauf:
Hi Mikael,
Am 10.05.24 um 11:45 schrieb Mikael Morin:
Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
I'll stop here...
Thanks. Go figure, I have no problem reproducing today.
It's PR99798 (and there is even a patch for it).
this patch has rotten a bit: the type of gfc_reluease_symbol
has changed to bool, this can be fixed.
Unfortunately, applying the patch does not remove the ICEs here...
Oops, I take that back! There was an error on my side applying the
patch; and now it does fix the ICEs after correcting that hickup
Now the PR99798 patch is ready to be pushed, but I won't be available
for a few days. We can finish our discussion on this topic afterwards.
Hello,
I'm coming back to this.
I think either one of Steve's patch or your variant in the PR is a
better fix for the ICE as a first step; they seem less fragile at least.
Then we can look at a possible reordering of conflict checks as with the
patch you originally submitted in this thread.
like the attached variant?
Harald
Mikael
From 68d73e6e2efa692afff10ea16eafb88236cbe69c Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Thu, 23 May 2024 21:13:00 +0200
Subject: [PATCH] Fortran: improve attribute conflict checking [PR93635]
gcc/fortran/ChangeLog:
PR fortran/93635
* symbol.cc (conflict_std): Helper function for reporting attribute
conflicts depending on the Fortran standard version.
(conf_std): Helper macro for checking standard-dependent conflicts.
(gfc_check_conflict): Use it.
gcc/testsuite/ChangeLog:
PR fortran/93635
* gfortran.dg/c-interop/c1255-2.f90: Adjust pattern.
* gfortran.dg/pr87907.f90: Likewise.
* gfortran.dg/pr93635.f90: New test.
Co-authored-by: Steven G. Kargl
---
gcc/fortran/symbol.cc | 63 +--
.../gfortran.dg/c-interop/c1255-2.f90 | 4 +-
gcc/testsuite/gfortran.dg/pr87907.f90 | 8 ++-
gcc/testsuite/gfortran.dg/pr93635.f90 | 19 ++
4 files changed, 54 insertions(+), 40 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr93635.f90
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 0a1646def67..5db3c887127 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -407,18 +407,36 @@ gfc_check_function_type (gfc_namespace *ns)
/ Symbol attribute stuff */
+/* Older standards produced conflicts for some attributes that are allowed
+ in newer standards. Check for the conflict and issue an error depending
+ on the standard in play. */
+
+static bool
+conflict_std (int standard, const char *a1, const char *a2, const char *name,
+ locus *where)
+{
+ if (name == NULL)
+{
+ return gfc_notify_std (standard, "%s attribute conflicts "
+ "with %s attribute at %L", a1, a2,
+ where);
+}
+ else
+{
+ return gfc_notify_std (standard, "%s attribute conflicts "
+ "with %s attribute in %qs at %L",
+ a1, a2, name, where);
+}
+}
+
/* This is a generic conflict-checker. We do this to avoid having a
single conflict in two places. */
#define conf(a, b) if (attr->a && attr->b) { a1 = a; a2 = b; goto conflict; }
#define conf2(a) if (attr->a) { a2 = a; goto conflict; }
-#define conf_std(a, b, std) if (attr->a && attr->b)\
- {\
-a1 = a;\
-a2 = b;\
-standard = std;\
-goto conflict_std;\
- }
+#define conf_std(a, b, std) if (attr->a && attr->b \
+&& !conflict_std (std, a, b, name, where)) \
+return false;
bool
gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
@@ -451,7 +469,6 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
"OACC DECLARE DEVICE_RESIDENT";
const char *a1, *a2;
- int standard;
if (attr->artificial)
return true;
@@ -460,20 +477,10 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
where = &gfc_current_locus;
if (attr->pointer && attr->intent != INTENT_UNKNOWN)
-{
- a1 = pointer;
- a2 = intent;
- standard = GFC_STD_F2003;
- goto conflict_std;
-}
+conf_std (pointer, intent, GFC_STD_F2003);
- if (attr->in_namelist && (attr->allocatable || attr->pointer))
-{
- a1 = in_namelist;
- a2 = attr->allocatable ? allocatable : pointer;
- standard = GFC_STD_F2003;
- goto conflict_std;
-}
+ conf_std (in_namelist, allocatable, GFC_STD_F2003);
+ conf_std (in_namelist, pointer, GFC_STD_F2003);
/* Check for attributes not allowed in a BLOCK DATA. */
if (gfc_current_state () == COMP_BLOCK_DATA)
@@ -922,20 +929,6 @@ conflict:
a1, a2,
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 23/05/2024 à 09:49, Mikael Morin a écrit : Le 13/05/2024 à 09:25, Mikael Morin a écrit : Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. Hello, I'm coming back to this. I think either one of Steve's patch or your variant in the PR is a better fix for the ICE as a first step; they seem less fragile at least. Then we can look at a possible reordering of conflict checks as with the patch you originally submitted in this thread. Replying to myself... It's not a great plan if we want to avoid unnecessary churn in the testsuite.
