search

patrikhuber / superviseddescent

C++11 implementation of the supervised descent optimisation method
http://patrikhuber.github.io/superviseddescent/
Apache License 2.0
402 stars 188 forks source link

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

Open euklid opened 7 years ago

euklid commented 7 years ago

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

also, see #27

patrikhuber commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@patrikhuber what are your thoughts?