[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/