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

Fix a linking problem by moving Stroke::save() and load() into header #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ghost
Copy link

@ghost ghost commented May 6, 2020

The template member functions Stroke::save and Stroke::load get called
via the serialize() function generated by boost's macro
BOOST_SERIALIZATION_SPLIT_MEMBER() in gesture.h. Since the definitions
of save()/load() are only available in gesture.cc, the compiler may
produce two versions of Stroke::serialize() -- one with save()/load()
inlined in gesture.o and one with calls to save()/load() in all other
referencing translation units. Since the compiler inlined Stroke::save()
and Stroke::load(), it will not export them in gesture.o (which is
legitimate, since the code only requests an export of
Stroke::serialize). As a result, some orders of object files can fail to
link, when the linker picks the version of Stroke::serialize() that
would call save()/load() (which are not available separately) instead of
the version with these functions inlined.

Avoid relying on this compiler- and optimization-level dependent
behavior by moving the definition of template member functions
Stroke::save() and Stroke::load() into gesture.h. As a side-effect, that
change unifies code style, since all other classes have their ::save()
and ::load() definitions in header files, too.

These link failures surfaced when building on s390x with -march=zEC12 or
later, and can be reproduced on x86_64 with gcc parameters
--param max-inline-insns-auto=80 --param inline-min-speedup=2

Signed-off-by: Marius Hillenbrand mhillen@linux.ibm.com

The template member functions Stroke::save and Stroke::load get called
via the serialize() function generated by boost's macro
BOOST_SERIALIZATION_SPLIT_MEMBER() in gesture.h.  Since the definitions
of save()/load() are only available in gesture.cc, the compiler may
produce two versions of Stroke::serialize() -- one with save()/load()
inlined in gesture.o and one with calls to save()/load() in all other
referencing translation units. Since the compiler inlined Stroke::save()
and Stroke::load(), it will not export them in gesture.o (which is
legitimate, since the code only requests an export of
Stroke::serialize). As a result, some orders of object files can fail to
link, when the linker picks the version of Stroke::serialize() that
would call save()/load() (which are not available separately) instead of
the version with these functions inlined.

Avoid relying on this compiler- and optimization-level dependent
behavior by moving the definition of template member functions
Stroke::save() and Stroke::load() into gesture.h. As a side-effect, that
change unifies code style, since all other classes have their ::save()
and ::load() definitions in header files, too.

These link failures surfaced when building on s390x with -march=zEC12 or
later, and can be reproduced on x86_64 with gcc parameters
    --param max-inline-insns-auto=80 --param inline-min-speedup=2

Signed-off-by: Marius Hillenbrand <mhillen@linux.ibm.com>
@bkuri
Copy link

bkuri commented Jul 7, 2020

@mhillenibm just wanted to let you know that I've been testing out this PR and it works just fine in my case. I've therefore submitted a gist to the easystroke-git AUR package page so that this gets considered for a future version of the package. Hope that's okay!

@revast
Copy link

revast commented Jun 6, 2022

Thanks, maybe try also @ https://github.com/berkeleybross/easy-gesture, its an actively maintained fork, 72 Commits ahead.

@bkuri, would be nice if you consider switching the AUR of easystroke to easy-gestures, or kindly ask the current maintainer to do so..

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

Successfully merging this pull request may close these issues.

3 participants