haskell / c2hs

c2hs is a pre-processor for Haskell FFI bindings to C libraries
http://hackage.haskell.org/package/c2hs
Other
199 stars 50 forks source link

The symbol `)' does not fit here. #199

Open claudeha opened 6 years ago

claudeha commented 6 years ago

Trying to cabal install -f bundled -f opengl fltkhs in Appveyor (Windows compiler-as-a-service, with ghc-8.2.2 tarball downloaded from haskell.org inside its MSYS environment). I get a failure from c2hs related to the intrin-impl.h bundled with the ghc distribution of mingw:

...
Installed language-c-0.7.1
Installed c2hs-0.28.3
....
Preprocessing library for fltkhs-0.5.4.4..
c2hs.exe: C header contains errors:
C:/msys64/opt/ghc-8.2.2/mingw/x86_64-w64-mingw32/include/psdk_inc/intrin-impl.h:763: (column 222) [ERROR]  >>> Syntax error !
  The symbol `)' does not fit here.
cabal: Leaving directory 'C:\msys64\tmp\cabal-tmp-2128\fltkhs-0.5.4.4'
cabal.exe: Error: some packages failed to install:
fltkhs-0.5.4.4-2soiFkskaE436y5UI0TpGW failed during the building phase. The exception was:
ExitFailure 1
Command exited with code 1

The problematic source file can be found inside the ghc-8.2.2 mingw tarball: https://downloads.haskell.org/~ghc/8.2.2/ghc-8.2.2-x86_64-unknown-mingw32.tar.xz Here is an excerpt with line numbers annotated:

...
/* this is line 241 */
/* This macro is used by _bittest & _bittest64

Parameters: (FunctionName, DataType, OffsetConstraint)
   FunctionName: Any valid function name
   DataType: __LONG32 or __int64
   Type: l, q
   OffsetConstraint: either "I" for 32bit data types or "J" for 64.

   */
#define __buildbittest(x, y, z, a) unsigned char x(const y *Base, y Offset) \
{ \
   unsigned char old; \
   __asm__ ("bt{" z " %[Offset],%[Base] | %[Base],%[Offset]}" __FLAGSET \
      : [old] __FLAGCONSTRAINT (old) \
      : [Offset] a "r" (Offset), [Base] "rm" (*Base) \
      : __FLAGCLOBBER2); \
   return old; \
}
/* this is line 260 */
...
/* this is line 759 */
#if __INTRINSIC_PROLOG(_bittest64)
__MINGW_EXTENSION unsigned char _bittest64(__int64 const *a, __int64 b);
__MINGW_EXTENSION __INTRINSICS_USEINLINE
__buildbittest(_bittest64, __int64, "q", "J")
#define __INTRINSIC_DEFINED__bittest64
#endif /* __INTRINSIC_PROLOG */
/* this is line 766 */
...

I don't have Windows at home so I don't know if it's Appveyor specific, or if it's a generic Windows issue. I also don't know if this should rather be an issue filed against fltkhs or language-c (or some other package).

claudeha commented 6 years ago

It works fine with ghc-7.10.3 from https://downloads.haskell.org/~ghc/7.10.3/ghc-7.10.3b-x86_64-unknown-mingw32.tar.xz , which has an earlier bundled mingw/gcc version.

claudeha commented 6 years ago

It also works fine with ghc-8.0.2 from https://downloads.haskell.org/~ghc/8.0.2/ghc-8.0.2-x86_64-unknown-mingw32-win10.tar.xz

deech commented 6 years ago

Thanks for this awesome bug report. Although I don't know much about inline ASM (__asm__ (...)) I was able to get c2hs to read this file when I passed cc-options: -D__FLAGCLOBBER2='"cc"' to the build options. So it seems the issue is __FLAGCLOBBER is not defined and c2hs fails to parse:

__asm__ ( <bunch-of-asm> 
               : )        // c2hs does not like a blank after the ':'

I'll have to look into it further. I don't know enough to know if this is illegal C, something language-c doesn't currently understand or an issue with c2hs itself.

In any case thanks digging through it and let me know if you're happy with the workaround for now.

maurges commented 4 years ago

Hello. I've got the same problem on GHC-8.10.1. This happens in my file where I #include <Windows.h>.

Unfortunately, the workaround doesn't work anymore, as __FLAGGLOBBER2 is now overwritten based on __GCC_ASM_FLAG_OUTPUTS__ in intrin-impl.h:77:

/* GCC v6 added support for outputting flags.  This allows better code to be
   produced for a number of intrinsics. */
#ifndef __GCC_ASM_FLAG_OUTPUTS__
#define __FLAGCONSTRAINT "=qm"
#define __FLAGSET "\n\tsetc %[old]"
#define __FLAGCLOBBER1 , "cc"
#define __FLAGCLOBBER2 "cc"
#else
#define __FLAGCONSTRAINT "=@ccc"
#define __FLAGSET
#define __FLAGCLOBBER1
#define __FLAGCLOBBER2
#endif

So the new workaround is to undefine the variable: cc-options: -U__GCC_ASM_FLAG_OUTPUTS__. But I wonder about that comment about better code for a number of intrinsics.