lv2 / pugl

A minimal portable API for embeddable GUIs
https://gitlab.com/lv2/pugl/
ISC License
174 stars 34 forks source link

Problem with PUGL_UNUSED #28

Closed marc-weber1 closed 4 years ago

marc-weber1 commented 4 years ago

Not sure why it's coded this way, but the code in types.h:

#if defined(__cplusplus) || defined(_MSC_VER)
#   define PUGL_UNUSED(name)
#elif defined(__GNUC__)
#   define PUGL_UNUSED(name) name##_unused __attribute__((__unused__))
#else
#   define PUGL_UNUSED(name)
#endif

Sometimes results in a macro in win.c being assigned to a blank space, leaving a function definition with no name for its argument:

puglGetNativeWorld(PuglWorld* PUGL_UNUSED(world))
{
    return GetModuleHandle(NULL);
}

Leading to the following stack trace:

"C:\Users\facade\Documents\Github\iPlug2\Examples\FacadeTest\projects\FacadeTest-vst2.vcxproj" (default target) (2) ->
(ClCompile target) ->
  C:\Users\facade\Documents\Github\iPlug2\Dependencies\IGraphics\pugl\pugl\detail\win.c(137,1): error C2055: expected f
ormal parameter list, not a type list [C:\Users\facade\Documents\Github\iPlug2\Examples\FacadeTest\projects\FacadeTest-
vst2.vcxproj]
  C:\Users\facade\Documents\Github\iPlug2\Dependencies\IGraphics\pugl\pugl\detail\win.c(732,1): error C2055: expected f
ormal parameter list, not a type list [C:\Users\facade\Documents\Github\iPlug2\Examples\FacadeTest\projects\FacadeTest-
vst2.vcxproj]
  C:\Users\facade\Documents\Github\iPlug2\Dependencies\IGraphics\pugl\pugl\detail\win.c(740,1): error C2055: expected f
ormal parameter list, not a type list [C:\Users\facade\Documents\Github\iPlug2\Examples\FacadeTest\projects\FacadeTest-
vst2.vcxproj]

Details: -Windows -64-bit -Visual studio/msbuild compiler

drobilla commented 4 years ago

Not sure why it's coded this way

This is to suppress unused parameter warnings. In C++, parameter names are not required so you can simply omit them, but in C you have to use an attribute.

Are you trying to build with MSVC in C mode? Since MS doesn't really support non-prehistoric C, the included build system uses /TP to build as C++ (pugl is written in the common subset of C and C++).

That said, this macro probably shouldn't have that assumption baked in. Can you try this:

#if defined(__cplusplus)
#   define PUGL_UNUSED(name)
#elif defined(__GNUC__)
#   define PUGL_UNUSED(name) name##_unused __attribute__((__unused__))
#else
#   define PUGL_UNUSED(name) name
#endif
marc-weber1 commented 4 years ago

Nope, but this works:

#if defined(__cplusplus) || defined(_MSC_VER)
#   define PUGL_UNUSED(name) name
#elif defined(__GNUC__)
#   define PUGL_UNUSED(name) name##_unused __attribute__((__unused__))
#else
#   define PUGL_UNUSED(name)
#endif

Forgot to mention I was compiling for c++, sorry

drobilla commented 4 years ago

Forgot to mention I was compiling for c++, sorry

I don't think you actually are. The documentation suggests this is a C error: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/compiler-error-c2055?view=vs-2019

I just checked myself and if I compile as C I get the same error. I know Pugl always builds with MSVC as C++, it's continually tested: https://gitlab.com/lv2/pugl/-/jobs/463726096

My fix works for me, so I'm assuming the above is a copy/paste error or something and I committed it (b7fef4609c3c07e7a117740affa9a7df10b6415d). If not we'll have to figure out what's going on here.

I'm surprised it actually builds as C with MSVC, but I guess if it works it works. This is untested and will at the very least generate a ton of warnings though, so figuring out how to switch it would be a good idea. MS has been pretty clear that they don't support modern C and writing in the common subset and building as C++ is the usually recommended workaround.

drobilla commented 4 years ago

Assuming fixed and closing since I can repro this issue on two machines and the above commit fixes it on both.

Free free to reopen if that's not the case.