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

make non-class and non-template functions inline #51

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

Conversation

euklid
Copy link

@euklid euklid commented Jul 16, 2017

also used the definition of the VL_INLINE macro from
http://www.vlfeat.org/api/portability.html#host-compiler-other

also, see #27

@patrikhuber
Copy link
Owner

Looks good to me, thank you!

One small question: Is now not VL_INLINE (meaning static __inline) used in hog.c, while for the same functions in hog.h, inline is used?

@euklid
Copy link
Author

euklid commented Jul 16, 2017

It's the other way round: VL_INLINE in hog.h and inline for hog.c.

Since it should be a header-only style library, the hog.c file is included at the end of the hog.h header.

According to https://stackoverflow.com/questions/7762731/whats-the-difference-between-static-and-static-inline-function , the static qualifies the affected functions to only be callable within the translation unit they are included in. Since the library is header only, they will be callable in any translation unit that will include the hog.h and because of the inline there won't be any problems with the ODR. IMHO, I don't see any problems with how it is now.

Keeping in mind that this is actually part of the bigger VLFeat library, there it was probably a good idea to keep some functions static inline in the header. I would guess, they wanted to avoid any linker errors within the VLFeat library and thus used some convenience functions as static inline when creating the several translation units of the different parts of the library. Probably, because there are no namespaces in C and using static seems to bea a way to avoid double definitions when linking. But this is only my guess. (My pure C is a bit rusty...)

@euklid
Copy link
Author

euklid commented Jul 22, 2017

@patrikhuber what are your thoughts?

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.

2 participants