HenrikBengtsson / affxparser

🔬 R package: This is the Bioconductor devel version of the affxparser package.
http://bioconductor.org/packages/devel/bioc/html/affxparser.html
7 stars 3 forks source link

For the records: Add rationale for src/_mingw.h #25

Open HenrikBengtsson opened 8 years ago

HenrikBengtsson commented 8 years ago

We should document/explain the rationale for having a src/_mingw.h in the package. I don't have notes from the original addition of this, but I'm pretty sure it is because of the following.

Background

When trying to build/install the package on Windows with our src/_mingw.h workaround, one gets:

                 from fusion_sdk/calvin_files/data/src/CDFData.cpp:21:
C:/Rtools/mingw_32/i686-w64-mingw32/include/servprov.h: In member
function 'HRESULT IServiceProvider::QueryService(const GUID&, Q**)':
C:/Rtools/mingw_32/i686-w64-mingw32/include/servprov.h:66:46: error:
expected primary-expression before ')' token
   return QueryService(guidService, __uuidof(Q), (void **)pp);
                                              ^
C:/Rtools/mingw_32/i686-w64-mingw32/include/servprov.h:66:46: error:
there are no arguments to '__uuidof' that depend on a template
parameter, so a declaration of '__uuidof' must be available [-fpermissive]
make[1]: *** [fusion_sdk/calvin_files/data/src/CDFData.o] Error 1

Troubleshooting

The Rtools tool chain includes:

These two differ between the gcc-4.6.3 and the gcc 4.9.3 toolchains, but they both share the following identical parts relevant to what we are using them for:

#ifdef _MSC_VER
#define USE___UUIDOF 1
#else
#define USE___UUIDOF 0
#endif

and

/* Macros for __uuidof template-based emulation */
#if defined(__cplusplus) && (USE___UUIDOF == 0)

#define __CRT_UUID_DECL(type,l,w1,w2,b1,b2,b3,b4,b5,b6,b7,b8)           \
    extern "C++" {                                                      \
    template<> inline const GUID &__mingw_uuidof<type>() {              \
        static const IID __uuid_inst = {l,w1,w2, {b1,b2,b3,b4,b5,b6,b7,b8}}; \
        return __uuid_inst;                                             \
    }                                                                   \
    template<> inline const GUID &__mingw_uuidof<type*>() {             \
        return __mingw_uuidof<type>();                                  \
    }                                                                   \
    }

#define __uuidof(type) __mingw_uuidof<__typeof(type)>()

#else

#define __CRT_UUID_DECL(type,l,w1,w2,b1,b2,b3,b4,b5,b6,b7,b8)

#endif

The cause of our problem seem to be that when building the package on Windows we end up with the "else" clause in the above macro. The reason for this is that USE___UUIDOF == 1, which in turn is because _MSC_VER is defined. The latter is defined because src/Markvars.win uses:

PKG_CPPFLAGS = \  
[...]
 -D_MSC_VER

First of all, if we don't define _MSC_VER, we get another error;

fusion_sdk/calvin_files/data/src/DataGroup.cpp:29:22: fatal error:
sys/mman.h: No such file or directory
 #include <sys/mman.h>
                      ^
compilation terminated.

I don't know the background/rational for _MSC_VER and whether it is important/critical for other reasons, so it could be that this "new" error is possible to fix. However, ...

Workaround

An alternative is to force USE___UUIDOF == 0 to get that [first clause]() in the if-else macro of src/_mingw.h. Unfortunately, it is not possible to do this from the src/Makevars.win file, because the src/_mingw.h file overrides it due to _MSC_VER. This is the reason why we have our own custom src/_mingw.h file(s) that uses:

#ifdef _MSC_VER
#define USE___UUIDOF 0
#else
#define USE___UUIDOF 0
#endif

(Sure, this could be replaced with a single line, but with the above the diff to the original ones is minimal; a single character).

kasperdanielhansen commented 8 years ago

Usually _MSC_VER is a check for the Visual C compiler. Unfortunately, it is hardcoded in a lot of places in Fusion. We could try to understand the issues a bit deeper.

HenrikBengtsson commented 8 years ago

In the meanwhile, an alternative "improvement" could be to see if it's possible to drop _mingw.h and override just the __CRT_UUID_DECL and __uuidof(type) macros, e.g.

#undef __CRT_UUID_DECL
#define __CRT_UUID_DECL(type,l,w1,w2,b1,b2,b3,b4,b5,b6,b7,b8)           \
    extern "C++" {                                                      \
    template<> inline const GUID &__mingw_uuidof<type>() {              \
        static const IID __uuid_inst = {l,w1,w2, {b1,b2,b3,b4,b5,b6,b7,b8}}; \
        return __uuid_inst;                                             \
    }                                                                   \
    template<> inline const GUID &__mingw_uuidof<type*>() {             \
        return __mingw_uuidof<type>();                                  \
    }                                                                   \
    }

#undef __uuidof
#define __uuidof(type) __mingw_uuidof<__typeof(type)>()

UPDATE: At least it's not as easy as putting the above in a uuid-fix.h and then updating Makevars.win:

%.o: %.cpp
    $(CXX) $(ALL_CPPFLAGS) $(ALL_CXXFLAGS) $(MYCXXFLAGS) --include _mingwd-uuid-fix.h -c $< -o $@

(it should be pretty clear by now I'm mostly guessing here)

Christopher-Yang-Chq commented 2 years ago

Dear HenrikBengtsson, i read this issue but i couldn't understand what this mean and how to fix it with re-write any code? I've post an issue lately, with my building error. Hope you have time to read that. Thanks a lot! @HenrikBengtsson