s-yata / marisa-trie

MARISA: Matching Algorithm with Recursively Implemented StorAge
Other
507 stars 89 forks source link

Warnings on 64-bit Windows #2

Closed superbobry closed 6 years ago

superbobry commented 8 years ago

Hello,

I'm one of the maintainers of the Python bindings to marisa-trie. Recently we started testing the code on Windows and came across a known issue with the 64-bit version. The library compiles with a number of invalid-cast warnings, but most of the tests fail to pass, which may indicate that despite successful compilation, the library is broken.

Here's a sample of the compilation warnings:

lib/marisa/grimoire/vector\bit-vector.cc(771) : warning C4267: 'argument' : conversion from 'size_t' to 'const marisa::UInt32', possible loss of data
lib/marisa/grimoire/vector\bit-vector.cc(776) : warning C4267: 'argument' : conversion from 'size_t' to 'const marisa::UInt32', possible loss of data
lib/marisa/grimoire/vector\bit-vector.cc(815) : warning C4267: 'argument' : conversion from 'size_t' to 'const marisa::UInt32', possible loss of data

You can see full compilation log on AppVeyor.

s-yata commented 8 years ago

Thank you for your report. Warnings to narrowing conversion may not be the cause of test failures. I'll check tests later.

s-yata commented 8 years ago

I added explicit typecasts to suppress warnings for narrowing conversion. Would you check whether it sill compiles with a number of invalid-cast warning?

In addition, I'm not sure but some errors might be due to the file mode. https://github.com/kmike/marisa-trie/issues/1#issuecomment-198243103

s-yata commented 8 years ago

Sorry, it must not be due to the file mode, because it passes tests on 32-bit environments.

s-yata commented 8 years ago

It seems that the following decrement is wrong. https://github.com/s-yata/marisa-trie/blob/master/lib/marisa/grimoire/vector/bit-vector.cc#L200

s-yata commented 8 years ago

Would you check if tests still fail or not with the latest version?

superbobry commented 8 years ago

Yes, with the master version of marisa-trie all the 64-bit tests succeed. Thank you very much!

Can you do a bugfix release?

s-yata commented 8 years ago

I'll do it in a few days.

superbobry commented 8 years ago

Just a friendly reminder that we're waiting on the new release to update the Python wrapper :)

s-yata commented 6 years ago

Sorry for my very very late reaction.

I've added a tag v0.2.5.

https://github.com/s-yata/marisa-trie/releases/tag/v0.2.5

superbobry commented 6 years ago

No worries. Thank you for releasing the fix!

s-yata commented 6 years ago

I've added a release v0.2.5.

https://github.com/s-yata/marisa-trie/releases/tag/v0.2.5

marisa-0.2.5.tar.gz is an archive generated by make dist.