search

moses-smt / mgiza

A word alignment tool based on famous GIZA++, extended to support multi-threading, resume training and incremental training.
161 stars 60 forks source link

Explicitly specify arguments' types of string::insert calls in model1 #6

Closed urikz closed 9 years ago

urikz commented 9 years ago

I was having troubles compiling the project, because of ambiguity with string::insert function. (See http://std.dkuug.dk/jtc1/sc22/wg21/docs/lwg-closed.html#84). This is just a small fix for it.

Test plan:
$ cmake .
$ make 
$ make install
aijanai commented 9 years ago

any regression test on other compilers/versions?

urikz commented 9 years ago

@aijanai, do you mean - if I tested it on other compilers or that I should write that kind of test? In the first case, I tried just gcc with and without C++11 (-DCMAKE_CXX_STANDARD=11) - it worked for both cases. The change shouldn't really affect anything =) In the second case, it's good to have that test, but it's slightly out of scope of the commit. Please, let me know what do you think.

kpu commented 9 years ago

The original code is a mess. . . integer to string conversion should be factored out as a function. But this pull request makes it better.

urikz commented 9 years ago

Thanks!

hieuhoang commented 9 years ago

it's works for most c++ compilers i can get my hands on. clang 7.0.0 gcc 4.8.4 gcc 4.6.4 Also works with old gcc 4.4.7 'cos I checked in https://github.com/moses-smt/mgiza/commit/a0361df1ef48cc14ddc58e64cb9ae9a0bcf21e18

aijanai commented 9 years ago

I asked if it was tested against other compilers, no need to build specialized tests since it would be out of scope. Fine for me