[PATCH] Fix select type parsing (PR fortran/78026)

Steve Kargl sgk@troutmask.apl.washington.edu
Thu Oct 27 19:45:00 GMT 2016


On Thu, Oct 27, 2016 at 09:07:49PM +0200, Jakub Jelinek wrote:
> 
> This PR has been reported as something related to OpenMP, but in the end
> I think it is unrelated, the bug I see is in the select type parsing.
> 
> The problem is that if select type is the very first stmt in the TU,
> we parse it and before actually accepting that ST_SELECT_TYPE, we perform
> various tasks needed for MAIN__ - e.g. assign gfc_current_ns proc_name.
> The problem is that when parsing select type, we create a nested
> gfc_namespace and so the name is assigned to this nested namespace rather
> than its parent (and various other operations done on this namespace).
> 
> Also, it seems decode_statement is grossly inefficient, for any of the
> statements handled in the /* General statement matching: ... */
> switch we allocate a new namespace:
>   gfc_current_ns = gfc_build_block_ns (gfc_current_ns);
>   match (NULL, gfc_match_select_type, ST_SELECT_TYPE);
>   ns = gfc_current_ns;
>   gfc_current_ns = gfc_current_ns->parent;
>   gfc_free_namespace (ns);
> only to free it a few lines later in the likely case that we aren't seeing
> select type.  And in select_type_38.f03 testcase below I've also tried
> to construct a testcase where it is invalid - because the gfc_match_label
> on the select_type already goes into the new namespace, no errors are
> diagnosed if the same label is used on multiple select type statements
> (but we diagnose same label on select case, if etc.).
> 
> So, the patch defers creating the new namespace until we really need it
> (thus, label is put still into the parent namespace, and only create
> the namespace after successfully parsing select type (, and then arrange
> either if we don't return MATCH_YES to free the namespace again in the
> gfc_match_select_type, or, when returning MATCH_YES, to keep the namespace
> only in new_st.ext.block.ns and not in gfc_current_ns.  Then, only in
> parse_select_type_block we switch back to that namespace.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 

Thanks for the detailed analysis.  The patch looks ok to me.
I would prefer functional and cosmetic changes to be committed 
separately, but in this case the cosmetic changes are small.

> +	{
> +	  std::swap (ns, gfc_current_ns);
> +	  gfc_free_namespace (ns);
> +	  return m;
> +	}

Not being C++ literate.  I assume that the above is essential

tmp_ns = ns
ns = gfc_currrent_ns
gfc_current_ns = tmp_ns
free(ns)

-- 
Steve



More information about the Gcc-patches mailing list