lsl_archived icon indicating copy to clipboard operation
lsl_archived copied to clipboard

the trouble with __func__

Open dmedine opened this issue 6 years ago • 9 comments

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.

dmedine avatar Nov 18 '18 16:11 dmedine

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.

mgrivich avatar Nov 18 '18 16:11 mgrivich

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.

cboulay avatar Nov 18 '18 16:11 cboulay

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.

tstenner avatar Nov 18 '18 19:11 tstenner

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

dmedine avatar Nov 18 '18 20:11 dmedine

@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.

cboulay avatar Nov 18 '18 20:11 cboulay

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.

dmedine avatar Nov 19 '18 03:11 dmedine

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

cboulay avatar Nov 19 '18 05:11 cboulay

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.

tstenner avatar Nov 19 '18 11:11 tstenner

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.

mgrivich avatar Nov 28 '18 23:11 mgrivich