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
