RFC: Front-end cleanups

Tobias Schlüter tobias.schlueter@physik.uni-muenchen.de
Wed Oct 3 12:00:00 GMT 2007


François-Xavier Coudert wrote:
> I'd like to do a few style/namespace cleanups in the front-end in the
> coming weeks. I'm writing here because technically, they might not be
> welcome in stage 3, but I consider them both needed and trivial enough
> that they can't mess things up. That's why I'm asking for your opinion
> here.

I'm in favor of that.  Stage 3 rules are there to make people fix bugs 
instead of developing new features.  Fixing maintenance issues is fixing 
future bugs.

Admittedly, I'm not completely neutral as I did commit a cleanup not too 
long ago, unaware that we had already moved to stage 3.

>   -- It moves the gfc_get_data_variable, gfc_get_data_value and
> gfc_get_data macros from gfortran.h into decl.c, because that's the
> only place they're ever used.

I believe the idea behind keeping these in gfortran.h was to keep all 
gfc_get_* macros in a single place.  I don't care much either way.

>   -- Change set_in_match_data into gfc_set_in_match_data, because it's
> not static and has its prototype in gfortran.h. Same for
> gfc_global_used.

You did it the other way round (which is the right way).

>   -- The non-static functions in data.c are only called from data.c
> itself and resolve.c. For this reason, I moved their prototype from
> libgfortran.h into a new data.h header, which is included by resolve.c

The corresponding Makefile bits are missing.

> How does that sound?

Good.

> Index: trans-expr.c
> ===================================================================
> --- trans-expr.c	(revision 128951)
> +++ trans-expr.c	(working copy)
> @@ -592,7 +592,7 @@ gfc_conv_unary_op (enum tree_code code, 
>       All other unary operators have an equivalent GIMPLE unary operator.  */
>    if (code == TRUTH_NOT_EXPR)
>      se->expr = build2 (EQ_EXPR, type, operand.expr,
> -		       build_int_cst (type, 0));
> +		       build_int_cst (TREE_TYPE (operand.expr), 0));
>    else
>      se->expr = build1 (code, type, operand.expr);
>  

(This hunk is unrelated to your other clenaups.)  Unless I'm mistaken, 
TREE_TYPE (operand.expr) == type here, as the necessary conversions 
should have been added during resolution.

> Index: trans-types.c
> ===================================================================
> --- trans-types.c	(revision 128951)
> +++ trans-types.c	(working copy)
> @@ -1178,7 +1178,7 @@ gfc_get_dtype (tree type)
>    if (size && INTEGER_CST_P (size))
>      {
>        if (tree_int_cst_lt (gfc_max_array_element_size, size))
> -	internal_error ("Array element size too big");
> +	gfc_error ("Array element size too big at %C");
>  
>        i += TREE_INT_CST_LOW (size) << GFC_DTYPE_SIZE_SHIFT;
>      }

This has nothing to do with your cleanups, but I think that change is 
wrong.  It's not an error the user made, but a shortcoming of the compiler.

Cheers,
- Tobi



More information about the Fortran mailing list