PING -- [PATCH, fortran] PR 85982 -- ICE in resolve_component

Message ID CAE4aFAm5gGyyLjkp=Ne8ea5V4X2=g=OmtGcUPb03unMB2DT1NQ@mail.gmail.com
State New
Headers show
Series
  • PING -- [PATCH, fortran] PR 85982 -- ICE in resolve_component
Related show

Commit Message

Iain Buclaw via Gcc-patches April 1, 2020, 5:19 p.m.
This simple patch was submitted some time ago (over 1 year), but got
lost without review. I have lately rebased and tested, and the patch
is still good. Is this OK to commit to trunk and for backport? I'd
like to port as far back as 7.

---
Fritz Reese

gcc/ChangeLog:
2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

       PR fortran/85982
       * fortran/decl.c (match_attr_spec): Lump COMP_STRUCTURE/COMP_MAP into
       attribute checking used by TYPE.

gcc/testsuite/ChangeLog:
2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

       PR fortran/85982
       * gfortran.dg/dec_structure_28.f90: New test.

Comments

Iain Buclaw via Gcc-patches April 1, 2020, 6 p.m. | #1
On Wed, Apr 1, 2020 at 1:19 PM Fritz Reese <fritzoreese@gmail.com> wrote:
[...]
> is still good. Is this OK to commit to trunk and for backport? I'd

> like to port as far back as 7.


I realized 7 branch is closed. I would backport to 8.

> gcc/testsuite/ChangeLog:

> 2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

>

>        PR fortran/85982

>        * gfortran.dg/dec_structure_28.f90: New test.


Forgot to include the testcase in the patch. Testcase is attached.
! { dg-do compile }
! { dg-options "-fdec-structure -fdec-static" }
!
! PR fortran/85982
!
! Test a regression wherein some component attributes were erroneously accepted
! within a DEC structure.
!

structure /s/
  integer :: a
  integer, intent(in) :: b ! { dg-error "is not allowed" }
  integer, intent(out) :: c ! { dg-error "is not allowed" }
  integer, intent(inout) :: d ! { dg-error "is not allowed" }
  integer, dimension(1,1) :: e ! OK
  integer, external, pointer :: f ! { dg-error "is not allowed" }
  integer, intrinsic :: f ! { dg-error "is not allowed" }
  integer, optional :: g ! { dg-error "is not allowed" }
  integer, parameter :: h ! { dg-error "is not allowed" }
  integer, protected :: i ! { dg-error "is not allowed" }
  integer, private :: j ! { dg-error "is not allowed" }
  integer, static :: k ! { dg-error "is not allowed" }
  integer, automatic :: l ! { dg-error "is not allowed" }
  integer, public :: m ! { dg-error "is not allowed" }
  integer, save :: n ! { dg-error "is not allowed" }
  integer, target :: o ! { dg-error "is not allowed" }
  integer, value :: p ! { dg-error "is not allowed" }
  integer, volatile :: q ! { dg-error "is not allowed" }
  integer, bind(c) :: r ! { dg-error "is not allowed" }
  integer, asynchronous :: t ! { dg-error "is not allowed" }
  character(len=3) :: v ! OK
  integer(kind=4) :: w ! OK
end structure

end
Tobias Burnus April 2, 2020, 7:50 a.m. | #2
In principle, I like the patch. However, I think one should
replace

gfc_error ("Attribute at %L is not allowed in a %s definition",
           …, state_name

by something like:

bool is_type = gfc_current_state () == COMP_DERIVED;
gfc_error (is_type ? G_("Attribute at %L is not allowed in a TYPE definition")
                   : G_("Attribute at %L is not allowed in a STRUCTURE definition"),
           …

Reason: (a) This makes translation simpler; e.g. 'structure' and 'type' have
different gender in several European languages. (Albeit in this case the
gender of 'definition' dominates in the cases I checked.)
(b) For TYPE, the string won't change such that the existing translations
still work – even if the update for STRUCTURE won't make it for the release.

Otherwise it looks good to me, including the test case in your follow-up email.

Cheers,

Tobias

On 4/1/20 7:19 PM, Fritz Reese via Fortran wrote:

> This simple patch was submitted some time ago (over 1 year), but got

> lost without review. I have lately rebased and tested, and the patch

> is still good. Is this OK to commit to trunk and for backport? I'd

> like to port as far back as 7.

>

> ---

> Fritz Reese

>

> gcc/ChangeLog:

> 2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

>

>         PR fortran/85982

>         * fortran/decl.c (match_attr_spec): Lump COMP_STRUCTURE/COMP_MAP into

>         attribute checking used by TYPE.

>

> gcc/testsuite/ChangeLog:

> 2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

>

>         PR fortran/85982

>         * gfortran.dg/dec_structure_28.f90: New test.

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Iain Buclaw via Gcc-patches April 2, 2020, 4:20 p.m. | #3
Tobias,

Thank you for the information. I didn't think about translations. I'll
post a new version and commit shortly.

Cheers,

Fritz

On Thu, Apr 2, 2020 at 3:50 AM Tobias Burnus <tobias@codesourcery.com> wrote:
>

> In principle, I like the patch. However, I think one should

> replace

>

> gfc_error ("Attribute at %L is not allowed in a %s definition",

>            …, state_name

>

> by something like:

>

> bool is_type = gfc_current_state () == COMP_DERIVED;

> gfc_error (is_type ? G_("Attribute at %L is not allowed in a TYPE definition")

>                    : G_("Attribute at %L is not allowed in a STRUCTURE definition"),

>            …

>

> Reason: (a) This makes translation simpler; e.g. 'structure' and 'type' have

> different gender in several European languages. (Albeit in this case the

> gender of 'definition' dominates in the cases I checked.)

> (b) For TYPE, the string won't change such that the existing translations

> still work – even if the update for STRUCTURE won't make it for the release.

>

> Otherwise it looks good to me, including the test case in your follow-up email.

>

> Cheers,

>

> Tobias

>

> On 4/1/20 7:19 PM, Fritz Reese via Fortran wrote:

>

> > This simple patch was submitted some time ago (over 1 year), but got

> > lost without review. I have lately rebased and tested, and the patch

> > is still good. Is this OK to commit to trunk and for backport? I'd

> > like to port as far back as 7.

> >

> > ---

> > Fritz Reese

> >

> > gcc/ChangeLog:

> > 2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

> >

> >         PR fortran/85982

> >         * fortran/decl.c (match_attr_spec): Lump COMP_STRUCTURE/COMP_MAP into

> >         attribute checking used by TYPE.

> >

> > gcc/testsuite/ChangeLog:

> > 2020-04-01  Fritz Reese  <foreese@gcc.gnu.org>

> >

> >         PR fortran/85982

> >         * gfortran.dg/dec_structure_28.f90: New test.

> -----------------

> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany

> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Patch

commit 03ade661deaa606b005304814be9f723158ed55f
Author: Fritz Reese <foreese@gcc.gnu.org>
Date:   Fri Mar 20 13:03:42 2020 -0400

    Fix for fortran/85982

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 2f458c4faac..05503b2d3c7 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -5408,15 +5408,18 @@  match_attr_spec (void)
       if (d == DECL_STATIC && seen[DECL_SAVE])
 	continue;
 
-      if (gfc_current_state () == COMP_DERIVED
+      if (gfc_comp_struct (gfc_current_state ())
 	  && d != DECL_DIMENSION && d != DECL_CODIMENSION
 	  && d != DECL_POINTER   && d != DECL_PRIVATE
 	  && d != DECL_PUBLIC && d != DECL_CONTIGUOUS && d != DECL_NONE)
 	{
+	  const char* const state_name = (gfc_current_state () == COMP_DERIVED
+					  ? "TYPE" : "STRUCTURE");
 	  if (d == DECL_ALLOCATABLE)
 	    {
 	      if (!gfc_notify_std (GFC_STD_F2003, "ALLOCATABLE "
-				   "attribute at %C in a TYPE definition"))
+				   "attribute at %C in a %s definition",
+				   state_name))
 		{
 		  m = MATCH_ERROR;
 		  goto cleanup;
@@ -5425,7 +5428,8 @@  match_attr_spec (void)
 	  else if (d == DECL_KIND)
 	    {
 	      if (!gfc_notify_std (GFC_STD_F2003, "KIND "
-				   "attribute at %C in a TYPE definition"))
+				   "attribute at %C in a %s definition",
+				   state_name))
 		{
 		  m = MATCH_ERROR;
 		  goto cleanup;
@@ -5449,7 +5453,8 @@  match_attr_spec (void)
 	  else if (d == DECL_LEN)
 	    {
 	      if (!gfc_notify_std (GFC_STD_F2003, "LEN "
-				   "attribute at %C in a TYPE definition"))
+				   "attribute at %C in a %s definition",
+				   state_name))
 		{
 		  m = MATCH_ERROR;
 		  goto cleanup;
@@ -5472,8 +5477,8 @@  match_attr_spec (void)
 	    }
 	  else
 	    {
-	      gfc_error ("Attribute at %L is not allowed in a TYPE definition",
-			 &seen_at[d]);
+	      gfc_error ("Attribute at %L is not allowed in a %s definition",
+			 &seen_at[d], state_name);
 	      m = MATCH_ERROR;
 	      goto cleanup;
 	    }