sccn / lsl_archived

Multi-modal time-synched data transmission over local network
242 stars 134 forks source link

the trouble with __func__ #361

Open dmedine opened 5 years ago

dmedine commented 5 years ago

I am using the VS2013 project (not CMake) to build LSL and it throws a '__func__ unidentified symbol' error. In common.h: https://github.com/sccn/labstreaminglayer/blob/master/LSL/liblsl/src/common.h#L13-L27.

However in my VS project, this ifdef isn't activated. I don't know why since I am using MSVC120 (which is less than 1600) but also, shouldn't it be #define __func__ __FUNCTION__ a la the second answer here: https://stackoverflow.com/questions/2281970/cross-platform-defining-define-for-macros-function-and-func

Replacing __func__ with __FUNCTION__ does compile and is probably more meaningful than "Unknown function" which is the current preprocessor value it gets.

Also, I am again frustrated by CMake because I unistalled the 38 gigs that is Qt5 on my laptop in order to make room for the notorious Windows 10 update that destroys computers with less than 45 gigs storage space free and I can't make CMake generate projects for building just liblsl without it---which should be a feature. This is a separate issue, though.

mgrivich commented 5 years ago

I can confirm that func is breaking the VS2013 project. I haven't investigated in any detail, but the current VS2013 project does compile in Visual Studio 2017, community edition (free).

It's on my to-do list to update the projects to work with new boost, which may change this behavior, but I may end up just dropping old versions of visual studio.

On 11/18/2018 8:10 AM, David Medine wrote:

I am using the VS2013 project (not CMake) to build LSL and it throws a 'func unidentified symbol' error. In common.h: https://github.com/sccn/labstreaminglayer/blob/master/LSL/liblsl/src/common.h#L13-L27.

However in my VS project, this ifdef isn't activated. I don't know why since I am using MSVC120 (which is less than 1600) but also, shouldn't it be #define func FUNCTION a la the second answer here: https://stackoverflow.com/questions/2281970/cross-platform-defining-define-for-macros-function-and-func

Replacing func with FUNCTION does compile and is probably more meaningful than "Unknown function" which is the current preprocessor value it gets.

Also, I am again frustrated by CMake because I unistalled the 38 gigs that is Qt5 on my laptop in order to make room for the notorious Windows 10 update that destroys computers with less than 45 gigs storage space free and I can't make CMake generate projects for building just liblsl without it---which should be a feature. This is a separate issue, though.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/361, or mute the thread https://github.com/notifications/unsubscribe-auth/AFC33cnjMTnFSVm2mboZMO2MGA8z6joAks5uwYaFgaJpZM4YoAq1.

cboulay commented 5 years ago

Also, I am again frustrated by CMake because I unistalled the 38 gigs that is Qt5 on my laptop in order to make room for the notorious Windows 10 update that destroys computers with less than 45 gigs storage space free and I can't make CMake generate projects for building just liblsl without it---which should be a feature. This is a separate issue, though.

You're right it should be. We should remove any mention of Qt from liblsl. There's a lot of qt-cmake boilerplate code and configuration shared across apps so it should exist somewhere all Apps can access. Moving it to labstreaminglayer/Apps/ should be ok. This isn't too difficult to do but it will require some testing, and some time.

tstenner commented 5 years ago

Strange, according to MSDN MSVC 2013 has __func__. Qt shouldn't be needed for liblsl at all, but it might tell you it couldn't find it but continue building until it finds the first app that actually depends on it. All the CMake/At stuff is already in the LSLCMake.cmake, and there was a good reason for it to be there and not in /Apps/, but I can't remember why at the moment.

dmedine commented 5 years ago

@cboulay, yeah. I know how complicated that is.

cboulay commented 5 years ago

@tstenner A quick guess is that we wanted out-of-tree builds to have access to both the cmake-qt macros/configs and the liblsl cmake macros/configs, and we didn't want each app to have to include two different "support" cmake files, especially when the second depends on the first and the location of the first is not determined until after some configuration steps. I bet we can find the right incantation of cmake variables and if/else clauses to make this work.

dmedine commented 5 years ago

Does anyone have a reason/not/ to change #define func "Unknown function" to #define func FUNCTION?

On 18.11.2018 21:54, Chadwick Boulay wrote:

@tstenner https://github.com/tstenner A quick guess is that we wanted out-of-tree builds to have access to both the cmake-qt macros/configs and the liblsl cmake macros/configs, and we didn't want each app to have to include two different "support" cmake files, especially when the second depends on the first and the location of the first is not determined until after some configuration steps. I bet we can find the right incantation of cmake variables and if/else clauses to make this work.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/361#issuecomment-439724983, or mute the thread https://github.com/notifications/unsubscribe-auth/ADch7vvSfvEsOzqz8YA7nEMfxQMfYDN-ks5uwckDgaJpZM4YoAq1.

cboulay commented 5 years ago

As long as it is embedded in an #ifdef _WIN32 (or similar) block then it's fine by me.

tstenner commented 5 years ago

I bet we can find the right incantation of cmake variables and if/else clauses to make this work.

I also decided against it, because right now app developers can download the compiled liblsl package, insert the ~10 lines of boilerplate in their CMakeLists.txt and start using LSL, with one part on /Apps/ and another somewhere else this would be more complicated.

We could check in LSLCMake.cmake if finding Qt is necessary, but we need to include it before finding Qt so the App CMakeLists would say 'I need Qt', include LSLCMake and then try to find Qt. I prefer the current approach where it says 'Qt not found, if this is a problem tell me where to find it' and in case of liblsl it then compiles it. With apps that use Qt, it will fail later on but then the exact reason and solution is printed a bit above it in the CMake output.

Does anyone have a reason/not/ to change #define func "Unknown function" to #define func FUNCTION?

As long as it is embedded in an #ifdef _WIN32` (or similar) block then it's fine by me.

I propose #ifdef __FUNCTION__.

Regarding VS2013: should I add a CI job for it?

I can't make CMake generate projects for building just liblsl without it

The simplest approach is to run CMake with the CMakeLists.txt in LSL/liblsl, at least that's what I do.

mgrivich commented 5 years ago

I am working on getting the (non CMAKE) Visual Studio 2013 project working, and I think adding

#if defined(_MSC_VER) && _MSC_VER < 1900
#define __func__ __FUNCTION__
#endif

to lsl_c.h is the way to go. I'm adding this to my commit to tstenner-remove_old_boost pull request.