Add inline with SNAPPY_ATTRIBUTE_ALWAYS_INLINE

[gdsotirov]

Add inline with SNAPPY_ATTRIBUTE_ALWAYS_INLINE on AdvanceToNextTag to fix the following compilation errors and a warning with GCC:

[  2%] Building CXX object CMakeFiles/snappy.dir/snappy.cc.o
/usr/bin/c++   -DHAVE_CONFIG_H -Dsnappy_EXPORTS -I/usr/src/tmp/snappy-1.1.9/build -I/usr/src/tmp/snappy-1.1.9  -O3 -march=i586 -mtune=i686 -Wall -Wextra -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC   -std=c++11 -o CMakeFiles/snappy.dir/snappy.cc.o -c /usr/src/tmp/snappy-1.1.9/snappy.cc
/usr/src/tmp/snappy-1.1.9/snappy.cc:1017:8: warning: always_inline function might not be inlinable [-Wattributes]
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^
/usr/src/tmp/snappy-1.1.9/snappy.cc: In function 'std::pair<const unsigned char*, int> snappy::DecompressBranchless(const uint8_t*, const uint8_t*, ptrdiff_t, T, ptrdiff_t) [with T = char*; uint8_t = unsigned char; ptrdiff_t = int]':
/usr/src/tmp/snappy-1.1.9/snappy.cc:1017:8: error: inlining failed in call to always_inline 'size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)': function body can be overwritten at link time
/usr/src/tmp/snappy-1.1.9/snappy.cc:1097:53: error: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                                                     ^
/usr/src/tmp/snappy-1.1.9/snappy.cc:1017:8: error: inlining failed in call to always_inline 'size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)': function body can be overwritten at link time
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^
/usr/src/tmp/snappy-1.1.9/snappy.cc:1097:53: error: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                                                     ^
/usr/src/tmp/snappy-1.1.9/snappy.cc:1017:8: error: inlining failed in call to always_inline 'size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)': function body can be overwritten at link time
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^
/usr/src/tmp/snappy-1.1.9/snappy.cc:1097:53: error: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                                                     ^
CMakeFiles/snappy.dir/build.make:137: recipe for target 'CMakeFiles/snappy.dir/snappy.cc.o' failed

Just like with other functions using SNAPPY_ATTRIBUTE_ALWAYS_INLINE macro (i.e. __attribute__((always_inline)) ) it is necessary to use C++ inline specifier.

[winterheart]

Please note, this error appears only with GCC and -DBUILD_SHARED_LIBS=ON. Need to add this configuration to Travis matrix to cover this problem.

[SpaceIm]

It also fails with -DBUILD_SHARED_LIBS=OFF actually. As well as clang < 9 or apple-clang < 11 with an other error.

[gdsotirov]

To clarify my build configuration that lead to reported errors was:

-DBUILD_SHARED_LIBS=ON \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_CXX_FLAGS="-O3" \
-DCMAKE_C_FLAGS="-O3" \
-DCMAKE_INSTALL_LIBDIR=/usr/lib \
-DCMAKE_INSTALL_PREFIX=/usr \
-DCMAKE_VERBOSE_MAKEFILE=TRUE

I'm not familiar with project's CI configuration (what compilers, OSes and configuration options are used), but I believe it is better to have code that supports most of the compilers, instead of just latest versions. Right now I'm compiling with GCC 5.5.0 for Slackware 14.2, but soon I'll be compiling the same with GCC 10.3 for Slackware 15.0 and there are variety of other configurations with other distributions.

[xkszltl]

Great! We hit this error as well in 1.1.9 with gcc-8 (but only for shared libs).
Build log was commented to the related commit 3b57165

[1/5] Building CXX object CMakeFiles/snappy.dir/snappy.cc.o
FAILED: CMakeFiles/snappy.dir/snappy.cc.o 
ccache /usr/bin/g++-8 -DHAVE_CONFIG_H -Dsnappy_EXPORTS -I. -I../ -fdebug-prefix-map='/tmp/scratch'='/usr/local/src' -g -Wall -Wextra -fno-exceptions -fno-rtti -mavx -mbmi2 -O3 -DNDEBUG -fPIC -std=c++11 -MD -MT CMakeFiles/snappy.dir/snappy.cc.o -MF CMakeFiles/snappy.dir/snappy.cc.o.d -o CMakeFiles/snappy.dir/snappy.cc.o -c ../snappy.cc
../snappy.cc:292:76: warning: ignoring attributes on template argument ‘__m128i’ {aka ‘__vector(2) long long int’} [-Wignored-attributes]
 static inline std::pair<__m128i /* pattern */, __m128i /* reshuffle_mask */>
                                                                            ^
../snappy.cc:292:76: warning: ignoring attributes on template argument ‘__m128i’ {aka ‘__vector(2) long long int’} [-Wignored-attributes]
../snappy.cc:1017:8: warning: always_inline function might not be inlinable [-Wattributes]
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^~~~~~~~~~~~~~~~
../snappy.cc: In function ‘std::pair<const unsigned char*, long int> snappy::DecompressBranchless(const uint8_t*, const uint8_t*, ptrdiff_t, T, ptrdiff_t) [with T = char*]’:
../snappy.cc:1017:8: error: inlining failed in call to always_inline ‘size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)’: function body can be overwritten at link time
../snappy.cc:1097:43: note: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                           ~~~~~~~~~~~~~~~~^~~~~~~~~~~
../snappy.cc:1017:8: error: inlining failed in call to always_inline ‘size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)’: function body can be overwritten at link time
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^~~~~~~~~~~~~~~~
../snappy.cc:1097:43: note: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                           ~~~~~~~~~~~~~~~~^~~~~~~~~~~
../snappy.cc:1017:8: error: inlining failed in call to always_inline ‘size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)’: function body can be overwritten at link time
 size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
        ^~~~~~~~~~~~~~~~
../snappy.cc:1097:43: note: called from here
         size_t tag_type = AdvanceToNextTag(&ip, &tag);
                           ~~~~~~~~~~~~~~~~^~~~~~~~~~~
ninja: build stopped: cannot make progress due to previous errors.

[xkszltl]

@pwnall Could you take a look? You recently break this function into 2 arch-optimized versions (b4888f7) but still have the issue.

[gdsotirov]

Although the total ignorance I updated the patch to avoid conflicts with the base branch.

[DmitryLukyanov]

Is there any updates here? We also see this when we try to build the snappy on linux

[andypost]

Current changes allow to build on linux

[gdsotirov]

@andypost Can you please, explain (i.e. what changed or give a commit reference)?

[andypost]

@gdsotirov I'm building it for Alpinelinux (musl) you can see it in https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/24894

[gdsotirov]

@andypost Ah, OK. I misunderstood you. I though you mean that "current changes" in this repository allow to build on Linux. Anyway, we'll apparently continue use the patch until upstream considers fixing the problem.

[Catwowo]

Why don't you fix it!! Debian gives different modification methods https://sources.debian.org/patches/snappy/1.1.9-2/0001-Add-inline-with-SNAPPY_ATTRIBUTE_ALWAYS_INLINE.patch/,and I also encountered this problem on MPIS.

[ffontaine]

Patch seems to be applied with d644ca8. I assume that this PR can be closed.

[gdsotirov]

Sure. It took more than 2 years to add one word in the code to fix compilation...