_Float128 handling seems to be still broken about language-c

[jchia]

So, it seems the bug can be resolved by one of the following:

• language-c should be specified as compatible only with GCC >= 7 or glibc < 2.26
• recognition of _Float128 added in #41 should be predicated on GCC version being >= 7
• language-c should define GCC version-related macros to pretend to be GCC 7 and above

[visq]

Ok, I do understand that language-c is now incompatible with code that defines a typedef _Float128.
C++ is not supported anyway, but gcc < 7 with a recent glibc might be a problem.
Possible solution would be to add _Float128 as a typedef, not a builtin token (type checker warning only).
This way you could also "predicate" the definition of Float128, not possible with the current implementation as a Lexer token.

[krakrjak]

The GLIBC announcement says:

* On ia64, powerpc64le, x86-32, and x86-64, the math library now implements
  128-bit floating point as defined by ISO/IEC/IEEE 60559:2011 (IEEE
  754-2008) and ISO/IEC TS 18661-3:2015.  Contributed by Paul E. Murphy,
  Gabriel F. T. Gomes, Tulio Magno Quites Machado Filho, and Joseph Myers.

  To compile programs that use this feature, the compiler must support
  128-bit floating point with the type name _Float128 (as defined by TS
  18661-3) or __float128 (the nonstandard name used by GCC for C++, and for
  C prior to version 7).  _GNU_SOURCE or __STDC_WANT_IEC_60559_TYPES_EXT__
  must be defined to make the new interfaces visible.

And after a closer look, it seems that assuming it as a basic_type_name in the Parser is wrong.

[peti]

Ping? Will this issue be resolved?

[visq]

This issue is hard to resolve if you want a "magic" solution that does not ask the language-c client to choose whether _Float128 should be a builtin typedef.

As said before, it would be feasible, although a major change, to remove _Float128 from the lexer and add it to the list of builtin typedefs, and then add a way to select whether _Float128 is a builtin typedef. The major change is that selecting different C dialects is not supported at the moment, so you need a new API. I guess that is not a realistic option.

For the users that are affected because they use glib >= 2.26 and gcc preprocessor < 7, it might be sufficient to call the gcc preprocessor in a way that it pretends to supports _Float128. I do not know if that is possible, but if it can be done via some argument parsed to the preprocessor, that would be a simple and pragmatic solution that does not even require a language-c change.

If it is not possible to workaround the issue with preprocessor options, I would suggest the first option suggested by @jchia, and simply state that code that re-typedefs _Float128 is not supported.

[krakrjak]

@visq I have been attempting to perform the fix suggested by @jchia:

recognition of _Float128 added in #41 should be predicated on GCC version being >= 7

My current approach is this diff in src/Language/C/Parser/Lexer.x:

+#if MIN_TOOL_VERSION_gcc(7, 0, 0)
 idkwtok ('_' : '_' : 'f' : 'l' : 'o' : 'a' : 't' : '1' : '2' : '8' : []) = tok 10 CTokFloat128
 idkwtok ('_' : 'F' : 'l' : 'o' : 'a' : 't' : '1' : '2' : '8' : []) = tok 9 CTokFloat128
+#endif

I am going to play with this approach. Are there any gotchas I need to be worried about? The above patch does build with gcc7. I don't really have a way of testing gcc <7 with newer glibc, that I can think of... Is there a distro I can whip up a VM with and dig into this scenario deeper? I know we can't really support both types of C code, but if there is a way to suppress lexing the token as a keyword in gcc<7 then I can feel this issue will be put to rest (hopefully).

[visq]

@krakrjak Good idea! You only want to #if _Float128, not __float128 though. If this works, let's merge it and publish a bugfix release.
In the c2hs issue, @peti reported the problem still affects Nix (haskell/c2hs#192 (comment)).

[lambdageek]

@krakrjak @visq This seems to be confusing the compiler that's installed at the time that language-c is built, with the compiler that's installed when language-c is running. Those are often the same, but they don't have to be.

I don't really have a better suggestion, however. (In my own downstream project I can afford to do some preprocessor hacks to define _Float128 away when gcc is new enough.)

[krakrjak]

@lambdageek you are so right... I'm really not sure how we would inject this detection purely at runtime...

[visq]

@lambdageek you are right, relying on build time gcc version is probably not ideal; if you update to gcc7 later, there is no simple way to specify that you need to update language-c, too.

We could introduce a Cabal flag for _Float128; then it is up to the package manager or the user to decide whether _Float128 is needed.
Alternatively, we could ask (affected) clients to rename _Float128 via #define.
Opinions?

[lambdageek]

I guess one thing that I'm unclear about is why _Float128 needs to be a lexer token. Can't it just be a tyident that's optionally available in Language.C.Parser.Builtin.builtinTypeNames? Is it really important that it has its own token in the lexer? (It will reduce as a typedef_type_specifier instead of a basic_type_specifier in the parser, but i'm not sure if there are any consequences to that.)

The next suggestion: can we make builtinTypeNames a function configurable at runtime and leave it to downstream projects to detect GCC and configure the parser appropriately?

As a practical matter, we can:

1. First change builtinTypeNames with a preprocessor pragma and do a minor version bump of language-c just to unblock anyone downstream hampered by GCC 7 + glibc 2.26
2. and then do a major version bump where the set of builtin type name is somehow configurable so downstreams can do a runtime check.

[visq]

@lambdageek No, it does not need its own lexer token. As I wrote before in this thread, adding it to builtin typedefs is a possible solution, but requires an API change to be runtime configurable.
The built-time configured workaround will work either way. If a cabal flag is sufficient, I'd go for that.

[jchia]

In glibc-2.27, there are now also _Float32, _Float64, _Float32x, _Float64x and _Float123x (I'm not sure whether this last one is a new addition). I don't think they were present in glibc-2.27. language-c fails to parse them on my Arch Linux machine. I think these are the same problem so I mention them here instead of a new issue.

I'm getting error messages that look like this:

    /usr/include/stdlib.h:140: (column 17) [ERROR]  >>> Syntax error !
      The symbol `strtof32' does not fit here.

This error is for code that looks like this:

#if __HAVE_FLOAT32 && __GLIBC_USE (IEC_60559_TYPES_EXT)
extern _Float32 strtof32 (const char *__restrict __nptr,
                          char **__restrict __endptr)
     __THROW __nonnull ((1));
#endif

[visq]

@jchia Thanks for the report. The missing types belong to #52, this ticket was about the new types conflicting with typedefs in case of an old gcc preprocessor.