Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the trouble with __func__ #361

Open
dmedine opened this issue Nov 18, 2018 · 9 comments
Open

the trouble with __func__ #361

dmedine opened this issue Nov 18, 2018 · 9 comments

Comments

@dmedine
Copy link
Contributor

dmedine commented Nov 18, 2018

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
Copy link
Collaborator

mgrivich commented Nov 18, 2018 via email

@cboulay
Copy link
Collaborator

cboulay commented Nov 18, 2018

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
Copy link
Collaborator

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
Copy link
Contributor Author

dmedine commented Nov 18, 2018

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

@cboulay
Copy link
Collaborator

cboulay commented Nov 18, 2018

@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
Copy link
Contributor Author

dmedine commented Nov 19, 2018 via email

@cboulay
Copy link
Collaborator

cboulay commented Nov 19, 2018

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

@tstenner
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants