[PATCH, fortran] Interoperability with C int128_t types

[FX]

> OK for mainline?

First, I noticed you haven't CC'ed the gcc-patches list: please do
that when you submit a patch.

Second, you still haven't taken into account the requests in my
previous review, I quote:

>> You also need to add that extension in our texinfo documentation.

This is important: gcc/fortran/gfortran.texi has a section listing
extensions supported by gfortran. You need to add an item for your
extension: it needs not be long, but it needs to be there and clear.

>> Finally, when asking for approval, you should say what testing has
been done and on which target(s).
>> (For example, "bootstrapped and regtested on x86_64-linux").

You still didn't say what amount of testing was done and on which
targets. I'd expect bootstrap and regtesting on at least one target
that has 128-bit integers and one target that doesn't.

Two minor issues:
  -- ChangeLog entries are usually not part of the diff, but attached
as is (or pasted in the message); it makes them easier to read.
  -- ChangeLog entries for different directories that have their own
ChangeLog (like, gcc/fortran/ and gcc/testsuite/) should be clearly
separated.


Now, I come to the patch itself. I asked you:

>> Also, what happens on platforms that don't have a 128-bit integer type?
>
> On platforms that do not support 128-bit integer, it will behave the same
> as if -std=f2003 is given.

I don't see how that is possible: I would rather guess that it
C_INT128_T will be defined and have the value -2, which is actually
what it ought to be (indicating a lack of support). But my question
was: how does the testcases behave on targets that don't have 128-bit
integer type? I think gfortran.dg/c_kind_int128_test2.f03 would FAIL,
which is not what we want. You should look at
gcc/testsuite/lib/target-supports.exp to see how
check_effective_target_fortran_large_real is defined (and used, for
example, in gfortran.dg/large_real_kind_1.f90) to avoid running tests
on targets that don't support them.

As I said above, you will need to regtest your patch on a platform
that don't have 128-bit integer support (or find someone who can do it
for you), to be sure we don't introduce new failures in the testsuite.

>  +static int
>  +gnu_only_isocbinding (const char *name)
>  +{
>  +#define NAMED_INTCST(a,b,c,d) \
>  +  if (!strcmp (b, name)) \
>  +    return (d == GFC_STD_GNU);
>  +#include "iso-c-binding.def"
>  +    return 0;
>  +}

I don't think we should have string comparisons if we can avoid it.
Also, at some point, we might want to have all the characteristics of
ISO_C_BINDING symbols in a array, so they can be accessed by their
numeric ID, but for now I think we should go with the following:

static int
std_for_isocbinding_symbol (int id)
{
  switch (id)
    {
#define NAMED_INTCST(a,b,c,d) \
      case a: \
        return d;
#include "iso-c-binding.def"
#undef NAMED_INTCST
    default:
      return GFC_STD_F2003;
    }
}

and then:

> +  if (gfc_notification_std (GFC_STD_GNU) == FAILURE
> +      && gnu_only_isocbinding (name))
> +    return;

will be changed into:

  if (gfc_notify_std (std_for_isocbinding_symbol (s),
                            "Extension: '%s' symbol in ISO_C_BINDING
intrinsic module at %C",
                            name) == FAILURE)
    return;

(this is untested, but you get the idea). This changes the error
message into actually indicating that this is an extension, giving a
hint to the user on how to allow it if he needs to.

Thanks,
FX

-- 
FX Coudert
http://www.homepages.ucl.ac.uk/~uccafco/