Different prototype and implementation for a function cause link error

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Try to build snappy with Solaris Studio compilers

What is the expected output? What do you see instead?
Link error:

Undefined           first referenced
 symbol                 in file
char*snappy::internal::CompressFragment(const char*,unsigned,char*,unsigned 
short*,const int) snappy_unittest-snappy_unittest.o

What version of the product are you using? On what operating system?
Latest code in trunk

Please provide any additional information below.

I've created a patch to fix the problem.

Original issue reported on code.google.com by trond.no...@gmail.com on 20 Jun 2011 at 6:17

Attachments:

• 0001-Fix-the-prototype-for-CompressFragment-so-it-match-t.patch

[GoogleCodeExporter]

Hi,

This is odd; the compiler should not be giving a link failure on this as of the 
C++ standard (if it does, a lot of legal code will break). What version are you 
using?

Original comment by se...@google.com on 20 Jun 2011 at 10:02

[GoogleCodeExporter]

I'm using:
CC: Sun C++ 5.11 SunOS_i386 2010/08/13

When I'm building I see stuff like:
libtool: link: ( cd ".libs" && rm -f "libsnappy.la" && ln -s "../libsnappy.la" 
"libsnappy.la" )
source='snappy_unittest.cc' object='snappy_unittest-snappy_unittest.o' 
libtool=no \
    DEPDIR=.deps depmode=none /bin/sh ./depcomp \
    CC -DHAVE_CONFIG_H -I.      -g -c -o snappy_unittest-snappy_unittest.o `test -f 'snappy_unittest.cc' || echo './'`snappy_unittest.cc
"snappy_unittest.cc", line 125: Warning: Identifier expected instead of "}".
1 Warning(s) detected.
source='snappy-test.cc' object='snappy_unittest-snappy-test.o' libtool=no \
    DEPDIR=.deps depmode=none /bin/sh ./depcomp \
    CC -DHAVE_CONFIG_H -I.      -g -c -o snappy_unittest-snappy-test.o `test -f 'snappy-test.cc' || echo './'`snappy-test.cc
/bin/sh ./libtool --tag=CXX   --mode=link CC  -g   -o snappy_unittest 
snappy_unittest-snappy_unittest.o snappy_unittest-snappy-test.o libsnappy.la 
-lz    
libtool: link: CC -g -o .libs/snappy_unittest snappy_unittest-snappy_unittest.o 
snappy_unittest-snappy-test.o  ./.libs/libsnappy.so -lz -R/usr/local/lib
Undefined           first referenced
 symbol                 in file
char*snappy::internal::CompressFragment(const char*,unsigned,char*,unsigned 
short*,const int) snappy_unittest-snappy_unittest.o
ld: fatal: symbol referencing errors. No output written to .libs/snappy_unittest
gmake[1]: *** [snappy_unittest] Error 2
gmake[1]: Leaving directory `/tmp/snappy-read-only'
gmake: *** [all] Error 2

If I'm inspecting the names in there I see:
trond@storm:2028> nm .libs/libsnappy.so | grep CompressFragment
[127]   |     19024|      2762|FUNC |GLOB |0    |12     
|__1cGsnappyIinternalQCompressFragment6FkpkckIpcpHki_6_
trond@storm:2029> nm snappy_unittest-snappy_unittest.o | grep CompressFragment
[290]   |         0|         0|FUNC |GLOB |0    |UNDEF  
|__1cGsnappyIinternalQCompressFragment6FpkcIpcpHki_4_

and if I try to demangle the functions I see:

trond@storm:2030> nm .libs/libsnappy.so | grep CompressFragment | c++filt
[127]   |     19024|      2762|FUNC |GLOB |0    |12     
|char*snappy::internal::CompressFragment(const char*const,const 
unsigned,char*,unsigned short*,const int)
trond@storm:2031> nm snappy_unittest-snappy_unittest.o | grep CompressFragment 
| c++filt
[290]   |         0|         0|FUNC |GLOB |0    |UNDEF  
|char*snappy::internal::CompressFragment(const char*,unsigned,char*,unsigned 
short*,const int)

I have to admit that I don't know the C++ standard good enough, but why use a 
different signature for the prototype and the implementation? This is the first 
time I've seen this "problem" so I'd like to know why doing so is a good thing?

Original comment by trond.no...@gmail.com on 20 Jun 2011 at 8:34

[GoogleCodeExporter]

[repaste; my answer went to the mailing list only]

For the same reason you don't put the entire implementation in the
header file, really. The fact that the int not changed inside the
implementation is irrelevant to the caller (the calling convention is
identical), and as such the caller should not care, nor should the
external user need to know about it.

I did some digging around, and seemingly this is just the Sun compiler
being known-broken (and Sun does not want to change it, as fixing this
would break backwards compatibility). I'm mildly tempted to just say
“use a standards-compliant C++ compiler”, but I guess the fix is
non-intrusive enough to include. I'd probably do it the other way
round, though, removing the consts in the .cc file; they shouldn't
affect code generation, and it gives more freedom to change the
implementation later without breaking the ABI (on this compiler).

Original comment by se...@google.com on 21 Jun 2011 at 12:14

[GoogleCodeExporter]

Hi,

Fixed in r44. Sorry about the late push.

Original comment by se...@google.com on 28 Jun 2011 at 11:41

• Changed state: Fixed