[Patch, Fortran] PR40569/40568: Support intrinsic module procedures (F2008)

[Daniel Kraft]

Hi Tobias,

Tobias Burnus wrote:
>  This patch add support for compiler_version and compiler_options, which 
> are part of F2008's ISO_FORTRAN_ENV. It also moves c_sizeof into the 
> module ISO_C_BINDINGS where it belongs to (before it was a normal 
> intrinsic).
> 
> TODO:
> a) With -Wall I get the warning that the intrinsic is already typed.
> b) compiler_options returns an empty string.
> 
> I plan to look into those issues as once as the patch is in.

this sounds reasonable.

> Build and currently regtesting on x86-64-linux.
> OK for the trunk?

Ok, but please consider the following:

+gfc_intrinsic_sym *
+gfc_intrinsic_function_by_id (gfc_isym_id id)
+{
+  gfc_intrinsic_sym *start = functions;
+  while (true)
+    {
+      if (id == start->id)
+	return start;
+
+      start++;
+    }
+
+  return NULL;
+}

I would like it better if an infinite loop were not possible here.  You 
could add a gcc_assert that at most nfunc iterations are done or the 
like; an assertion-failure seems much cleaner to me that looping 
forever, should the ID be not found (for whatever reason).

-  return (sym == NULL) ? 0 : sym->generic;
+  return (sym == NULL || sym->from_module) ? 0 : sym->generic;

(At more than one place.)  While you're at it, you could change "sym == 
NULL" to "!sym" as in other places in your patch.

+  if (tmp_symtree != NULL)
+    {
+      if (strcmp (modname, tmp_symtree->n.sym->module) == 0)
+        return;
+      else
+        gfc_error ("Symbol '%s' already declared", name);
+    }

Maybe get right of the "!= NULL".  And in any case I'd like it better if 
you did not "else" after the return.

+#define NAMED_FUNCTION(a,b,c,d) \
+  strncpy (c_interop_kinds_table[a].name, b, strlen(b) + 1); \
+  c_interop_kinds_table[a].f90_type = BT_PROCEDURE; \
+  c_interop_kinds_table[a].value = c;
+#include "iso-c-binding.def"

What about adding a corresponding #undef?

+  if ((sym->intmod_sym_id
+       && (isym
+	   = gfc_intrinsic_function_by_id ((gfc_isym_id) sym->intmod_sym_id)))
+      || (isym = gfc_find_function (sym->name)))

Hm -- I wonder whether this is correct.  Do you possibly want to call 
gfc_find_function *only* when sym->intmod_sym_id is NULL?  Or also in 
the case that sym->intmod_sym_id is set but gfc_intrinsic_function_by_id 
fails?  To me this seems to do the latter, but I'm not sure if that is 
correct.  (But if this is indeed what you want to do, then it is of 
course ok.)

Thanks for the patch!

Yours,
Daniel

-- 
http://www.pro-vegan.info/
--
Done:  Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz
To go: Hea-Mon-Pri