Re: [Patch, Fortran] RFC: Exit from non-loop constructs

[Mikael Morin <mikael dot morin at sfr dot fr>]

Le 30.08.2010 20:31, Daniel Kraft a écrit :
> Hi,
>
> the attached patch implements the Fortran 2008 feature of EXITs from
> non-loop named constructs (like IF or BLOCK). It seems to work
> basically, but I'd like to get some input.
>
> In particular, the two things marked XXX in the patch -- what do you
> think about those? (Both things are basically "boolean": I see two
> possible approaches, and just need to find out which one is the "correct".)
See below
> This patch regtests nearly fine on GNU/Linux-x86-32, but float128_1.f90
> fails ("Kind 16 not supported.") -- I've no idea what's going wrong
> there on my system, but it is probably not related.
Same here
> However, I also see a failure for gfortran.dg/gomp/pr41344.f: It seems
> to be that the error that is expected on line 10 appears on line 7
> instead. This is probably caused by my patch, although I have no idea
> what the problem could be... Any ideas? As this seems to be a middle-end
> error, I'm particular at a loss. Maybe I can work this out tomorrow
> myself, but I'd appreciate any thoughts.
I've looked at it and can't make any sense of it either.
The if clause has the wrong location at the gimple level, but setting 
the input_location to code->loc at trans stage doesn't help.

> I'm not asking for review yet, but will resubmit a new patch when the
> issues mentioned above are cleared.
>
> Yours,
> Daniel

> Index: gcc/fortran/match.c
> ===================================================================
> --- gcc/fortran/match.c	(revision 163637)
> +++ gcc/fortran/match.c	(working copy)
> @@ -2034,7 +2034,7 @@ match_exit_cycle (gfc_statement st, gfc_
>        sym = stree->n.sym;
>        if (sym->attr.flavor != FL_LABEL)
>  	{
> -	  gfc_error ("Name '%s' in %s statement at %C is not a loop name",
> +	  gfc_error ("Name '%s' in %s statement at %C is not a construct name",
>  		     name, gfc_ascii_statement (st));
>  	  return MATCH_ERROR;
>  	}
> @@ -2042,9 +2042,7 @@ match_exit_cycle (gfc_statement st, gfc_
>
>    /* Find the loop specified by the label (or lack of a label).  */
>    for (o = NULL, p = gfc_state_stack; p; p = p->previous)
> -    if (p->state == COMP_DO && (sym == NULL || sym == p->sym))
> -      break;
> -    else if (o == NULL && p->state == COMP_OMP_STRUCTURED_BLOCK)
> +    if (o == NULL && p->state == COMP_OMP_STRUCTURED_BLOCK)
>        o = p;
>      else if (p->state == COMP_CRITICAL)
>        {
> @@ -2052,19 +2050,57 @@ match_exit_cycle (gfc_statement st, gfc_
>  		  gfc_ascii_statement (st));
>  	return MATCH_ERROR;
>        }
> +    else if ((sym && sym == p->sym) || (!sym && p->state == COMP_DO))
> +      break;
>
>    if (p == NULL)
>      {
>        if (sym == NULL)
> -	gfc_error ("%s statement at %C is not within a loop",
> +	gfc_error ("%s statement at %C is not within a construct",
>  		   gfc_ascii_statement (st));
>        else
> -	gfc_error ("%s statement at %C is not within loop '%s'",
> +	gfc_error ("%s statement at %C is not within construct '%s'",
>  		   gfc_ascii_statement (st), sym->name);
>
>        return MATCH_ERROR;
>      }
>
> +  /* Special checks for EXIT from non-loop constructs.  */
> +  switch (p->state)
> +    {
> +    case COMP_DO:
> +      break;
> +
> +    case COMP_CRITICAL:
> +      /* This is already handled above.  */
> +      gcc_unreachable ();
> +
> +    case COMP_ASSOCIATE:
> +    case COMP_BLOCK:
> +    case COMP_IF:
> +    case COMP_SELECT:
> +    case COMP_SELECT_TYPE:
> +      gcc_assert (sym);
> +      if (op != EXEC_EXIT)
> +	{
> +	  gfc_error ("%s statement at %C is not applicable to construct '%s'",
> +		     gfc_ascii_statement (st), sym->name);
This is for cycle statement, right ?
I would explicitly assert op == EXEC_CYCLE and explain why it is not 
applicable (construct is not a loop).
Something like:
... not applicable to non-loop construct '%s'
> +	  return MATCH_ERROR;
> +	}
> +      if (gfc_notify_std (GFC_STD_F2008, "Fortran 2008: EXIT with no"
> +			  " do-construct-name at %C") == FAILURE)
> +	return MATCH_ERROR;
> +      break;
> +
> +    default:
> +      /* XXX: Could this be any other construct at all?  Can we
> +	 gcc_unreachable() here?  If not, which construct to use
> +	 in the test-case to verify this error?  */
I think this is unreachable, but I can't tell for sure.
Anyway let's keep an error as it is safer.

      program main

          implicit none

          integer :: foo
          integer :: bar(2)

          baz: where ([ .true., .true. ])
            bar = 0
            exit baz
          end where baz

      end program

I can trigger the error with the program above, so it is not unreachable 
after all.
Even without this error, the offending line is rejected later anyway.


> +      gfc_error ("%s statement at %C is not applicable to construct '%s'",
> +		 gfc_ascii_statement (st), sym->name);
> +      return MATCH_ERROR;
> +    }
> +
>    if (o != NULL)
>      {
>        gfc_error ("%s statement at %C leaving OpenMP structured block",
> @@ -2096,13 +2132,14 @@ match_exit_cycle (gfc_statement st, gfc_
>  	}
>        if (st == ST_CYCLE && cnt < collapse)
>  	{
> -	  gfc_error ("CYCLE statement at %C to non-innermost collapsed !$OMP DO loop");
> +	  gfc_error ("CYCLE statement at %C to non-innermost collapsed"
> +		     " !$OMP DO loop");
>  	  return MATCH_ERROR;
>  	}
>      }
>
> -  /* Save the first statement in the loop - needed by the backend.  */
> -  new_st.ext.whichloop = p->head;
> +  /* Save the first statement in the construct - needed by the backend.  */
> +  new_st.ext.which_construct = p->construct;
>
>    new_st.op = op;
>
> Index: gcc/fortran/parse.c
> ===================================================================
> --- gcc/fortran/parse.c	(revision 163637)
> +++ gcc/fortran/parse.c	(working copy)
> @@ -989,6 +989,16 @@ push_state (gfc_state_data *p, gfc_compi
>    p->sym = sym;
>    p->head = p->tail = NULL;
>    p->do_variable = NULL;
> +
> +  /* If this the state of a construct like BLOCK, DO or IF, the corresponding
> +     construct statement was accepted right before pushing the state.  Thus,
> +     the construct's gfc_code is available as tail of the parent state.  */
> +  /* XXX: Is there never a NULL previous state?  I.e., should this be rather
> +     put in if(gfc_state_stack)... instead of the assertion?  But it seems
> +     to regtest fine.  */
gfc_parse_file has at the very beginning :
  gfc_state_stack = &top;

So I think that it is indeed guaranteed to be non-null.
> +  gcc_assert (gfc_state_stack);
> +  p->construct = gfc_state_stack->tail;
> +
>    gfc_state_stack = p;
>  }