redpony / cdec

Decoder, aligner, and model optimizer for statistical machine translation and other structured prediction models based on (mostly) context-free formalisms
http://cdec-decoder.org/
Apache License 2.0
183 stars 77 forks source link

Changed implicit casts (from void* to gzFile_s*) in klm #6

Closed neubig closed 12 years ago

neubig commented 12 years ago

With the current version I get a compile error in klm/util/file_piece.cc

filepiece.cc: In constructor ‘util::GZException::GZException(void)’: filepiece.cc:34: error: invalid conversion from ‘void’ to ‘gzFiles’ filepiece.cc:34: error: initializing argument 1 of ‘const char gzerror(gzFiles, int_)’ file_piece.cc: In destructor ‘util::FilePiece::~FilePiece()’: filepiece.cc:59: error: invalid conversion from ‘void’ to ‘gzFiles’ file_piece.cc:59: error: initializing argument 1 of ‘int gzclose(gzFiles)’ file_piece.cc: In member function ‘void util::FilePiece::ReadShift()’: filepiece.cc:299: error: invalid conversion from ‘void’ to ‘gzFiles’ file_piece.cc:299: error: initializing argument 1 of ‘int gzread(gzFiles, void*, unsigned int)’

The compiler (g++ (Debian 4.4.5-8) 4.4.5) is getting mad at me for doing an implicit cast, so this pull request fixes this by adding explicit casts. The strange thing is, the same compiler works fine when compiling the kenlm I pulled directly from git, and I wasn't immediately able to debug why... Feel free to deny the request and fix it upstream, whatever works best.

redpony commented 12 years ago

Hi Graham, It looks like the change broke a couple things. Kenneth probably has a better idea for how he wants this fixed in any case (his code typically compiles everywhere, so I'm sure this is of interest to him).

cmu-432359:util cdyer$ make g++ -DHAVE_CONFIG_H -I. -I../.. -W -Wall -Wno-sign-compare -I.. -I/Users/cdyer/software/prefix/include -I/Users/cdyer/software/eigen-eigen-13a11181fc5a -DHAVE_CONFIG_H -g -O2 -MT file_piece.o -MD -MP -MF .deps/file_piece.Tpo -c -o file_piece.o file_piece.cc file_piece.cc: In constructor ‘util::GZException::GZException(void*)’: file_piece.cc:34:21: error: ‘gzFile_s’ was not declared in this scope file_piece.cc:34:30: error: expected primary-expression before ‘)’ token file_piece.cc: At global scope: file_piece.cc:31:1: warning: unused parameter ‘file’ [-Wunused-parameter] file_piece.cc: In destructor ‘util::FilePiece::~FilePiece()’: file_piece.cc:59:33: error: ‘gzFile_s’ was not declared in this scope file_piece.cc:59:42: error: expected primary-expression before ‘)’ token file_piece.cc: In member function ‘void util::FilePiece::ReadShift()’: file_piece.cc:299:25: error: ‘gzFile_s’ was not declared in this scope file_piece.cc:299:34: error: expected primary-expression before ‘)’ token

-Chris

On Mon, May 28, 2012 at 4:03 AM, Graham Neubig reply@reply.github.com wrote:

With the current version I get a compile error in klm/util/file_piece.cc

filepiece.cc: In constructor ‘util::GZException::GZException(void)’: filepiece.cc:34: error: invalid conversion from ‘void’ to ‘gzFiles’ filepiece.cc:34: error:   initializing argument 1 of ‘const char gzerror(gzFiles, int_)’ file_piece.cc: In destructor ‘util::FilePiece::~FilePiece()’: filepiece.cc:59: error: invalid conversion from ‘void’ to ‘gzFiles’ file_piece.cc:59: error:   initializing argument 1 of ‘int gzclose(gzFiles)’ file_piece.cc: In member function ‘void util::FilePiece::ReadShift()’: filepiece.cc:299: error: invalid conversion from ‘void’ to ‘gzFiles’ file_piece.cc:299: error:   initializing argument 1 of ‘int gzread(gzFiles, void*, unsigned int)’

The compiler (g++ (Debian 4.4.5-8) 4.4.5) is getting mad at me for doing an implicit cast, so this pull request fixes this by adding explicit casts. The strange thing is, the same compiler works fine when compiling the kenlm I pulled directly from git, and I wasn't immediately able to debug why... Feel free to deny the request and fix it upstream, whatever works best.

You can merge this Pull Request by running:

 git pull https://github.com/neubig/cdec fix_ptr_cast_in_klm

Or you can view, comment on it, or merge it online at:

 https://github.com/redpony/cdec/pull/6

-- Commit Summary --

  • Changed implicit casts (from void* to gzFile_s*) to explicit casts to fix a compile error

-- File Changes --

M klm/util/file_piece.cc (6)

-- Patch Links --

 https://github.com/redpony/cdec/pull/6.patch  https://github.com/redpony/cdec/pull/6.diff


Reply to this email directly or view it on GitHub: https://github.com/redpony/cdec/pull/6

neubig commented 12 years ago

Hmm, I didn't consider that adding the cast would break something elsewhere, but there might be some strange version clash going on at my side. I'll try to reproduce this in kenlm itself and close the pull request here.

kpu commented 12 years ago

Sorry, I had this in stable kenlm but steamrolled the rest costs over that. I've ported the fix to rest costs and put that in cdec.

neubig commented 12 years ago

Thanks a lot! I confirmed that it works with the most recent pull.