search

mischasan / aho-corasick

A-C implementation in "C". Tight-packed (interleaved) state-transition matrix -- as fast as it gets, as small as it gets.
GNU Lesser General Public License v3.0
147 stars 41 forks source link

error when compile with 64bit state ids (ACISM_SIZE 8) in windows #34

Open changnet opened 3 years ago

changnet commented 3 years ago 
typedef uint64_t TRAN, STATE, STRNO;

enum { 
    IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),
    IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),
    T_FLAGS   = IS_MATCH | IS_SUFFIX
};

Visual Studio(VS2013 VS2015 VS2019) warn about warning C4309: 'initializing' : truncation of constant value and actually truncate it，the value become zero. 1

in modern gcc(gcc 8 or gcc 10), if compile with -pedantic will given a warning. anyway, the value is expected.

@ MSYS /tmp
$ gcc -Wall -pedantic test.c
test.c:6:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic]
    6 |     IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),
      |                 ^
test.c:7:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic]
    7 |     IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),
      |                 ^
test.c:8:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic]
    8 |     T_FLAGS   = IS_MATCH | IS_SUFFIX
      |                 ^~~~~~~~

@ MSYS /tmp
$ ./a.exe
Hello World 9223372036854775808

maybe using static const instead ?

static const TRAN IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1);
static const TRAN IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2);
static const TRAN T_FLAGS   = ((TRAN)1 << (8*sizeof(TRAN) - 1)) | ((TRAN)1 << (8*sizeof(TRAN) - 1));
mischasan commented 3 years ago

Thanks! I normally build across several Unix plats, not Msft. So ... time for s/enum/int/g This applies to VS2013 and newer, then? I can test on VS2019

On 15/01/2021, changnet notifications@github.com wrote:


typedef uint64_t TRAN, STATE, STRNO;

enum {

    IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

    IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

    T_FLAGS   = IS_MATCH | IS_SUFFIX

};

Visual Studio(VS2013 VS2015 VS2019) warn about warning C4309: 'initializing' : truncation of constant value and actually truncate it，the value become zero.

1

in modern gcc(gcc 8 or gcc 10), if compile with -pedantic will given a warning. anyway, the value is expected.


@ MSYS /tmp

$ gcc -Wall -pedantic test.c

test.c:6:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    6 |     IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

      |                 ^

test.c:7:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    7 |     IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

      |                 ^

test.c:8:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    8 |     T_FLAGS   = IS_MATCH | IS_SUFFIX

      |                 ^~~~~~~~

@ MSYS /tmp

$ ./a.exe

Hello World 9223372036854775808

maybe using static const instead ?


static const TRAN IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1);

static const TRAN IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2);

static const TRAN T_FLAGS   = ((TRAN)1 << (8*sizeof(TRAN) - 1)) | ((TRAN)1
<< (8*sizeof(TRAN) - 1));

--

You are receiving this because you are subscribed to this thread.

Reply to this email directly or view it on GitHub:

https://github.com/mischasan/aho-corasick/issues/34

-- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

mischasan commented 3 years ago

And... I see that you answered my last question already. Will read the full text.

On 16/01/2021, Mischa Sandberg mischa@sandberg.ca wrote:

Thanks! I normally build across several Unix plats, not Msft. So ... time for s/enum/int/g This applies to VS2013 and newer, then? I can test on VS2019

On 15/01/2021, changnet notifications@github.com wrote:


typedef uint64_t TRAN, STATE, STRNO;

enum {

    IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

    IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

    T_FLAGS   = IS_MATCH | IS_SUFFIX

};

Visual Studio(VS2013 VS2015 VS2019) warn about warning C4309: 'initializing' : truncation of constant value and actually truncate it，the value become zero.

1

in modern gcc(gcc 8 or gcc 10), if compile with -pedantic will given a warning. anyway, the value is expected.


@ MSYS /tmp

$ gcc -Wall -pedantic test.c

test.c:6:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    6 |     IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

      |                 ^

test.c:7:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    7 |     IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

      |                 ^

test.c:8:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    8 |     T_FLAGS   = IS_MATCH | IS_SUFFIX

      |                 ^~~~~~~~

@ MSYS /tmp

$ ./a.exe

Hello World 9223372036854775808

maybe using static const instead ?


static const TRAN IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1);

static const TRAN IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2);

static const TRAN T_FLAGS   = ((TRAN)1 << (8*sizeof(TRAN) - 1)) |
((TRAN)1
<< (8*sizeof(TRAN) - 1));

--

You are receiving this because you are subscribed to this thread.

Reply to this email directly or view it on GitHub:

https://github.com/mischasan/aho-corasick/issues/34

-- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

-- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

zxxunkxzhx commented 3 years ago

I write it as

define IS_MATCH ( (TRAN)1 << (8 * sizeof(TRAN) - 1) )

define IS_SUFFIX ( (TRAN)1 << (8 * sizeof(TRAN) - 2) )

define T_FLAGS ( IS_MATCH | IS_SUFFIX )

and found it's okay

mischasan commented 3 years ago

Yep, that looks like the right answer. Did you push it?

On 22/02/2021, zxxunkxzhx notifications@github.com wrote:

I write it as

define IS_MATCH ( (TRAN)1 << (8 * sizeof(TRAN) - 1) )

define IS_SUFFIX ( (TRAN)1 << (8 * sizeof(TRAN) - 2) )

define T_FLAGS ( IS_MATCH | IS_SUFFIX )

and found it's okay

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/mischasan/aho-corasick/issues/34#issuecomment-783657586

-- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

mischasan commented 3 years ago

curious: why not static const TRAN T_FLAGS (IS_MATCH | IS_SUFFIX) ?

On 15/01/2021, changnet notifications@github.com wrote:


typedef uint64_t TRAN, STATE, STRNO;

enum {

    IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

    IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

    T_FLAGS   = IS_MATCH | IS_SUFFIX

};

Visual Studio(VS2013 VS2015 VS2019) warn about warning C4309: 'initializing' : truncation of constant value and actually truncate it，the value become zero.

1

in modern gcc(gcc 8 or gcc 10), if compile with -pedantic will given a warning. anyway, the value is expected.


@ MSYS /tmp

$ gcc -Wall -pedantic test.c

test.c:6:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    6 |     IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1),

      |                 ^

test.c:7:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    7 |     IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2),

      |                 ^

test.c:8:17: 警告：ISO C restricts enumerator values to range of 'int'
[-Wpedantic]

    8 |     T_FLAGS   = IS_MATCH | IS_SUFFIX

      |                 ^~~~~~~~

@ MSYS /tmp

$ ./a.exe

Hello World 9223372036854775808

maybe using static const instead ?


static const TRAN IS_MATCH  = (TRAN)1 << (8*sizeof(TRAN) - 1);

static const TRAN IS_SUFFIX = (TRAN)1 << (8*sizeof(TRAN) - 2);

static const TRAN T_FLAGS   = ((TRAN)1 << (8*sizeof(TRAN) - 1)) | ((TRAN)1
<< (8*sizeof(TRAN) - 1));

--

You are receiving this because you are subscribed to this thread.

Reply to this email directly or view it on GitHub:

https://github.com/mischasan/aho-corasick/issues/34

-- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

zxxunkxzhx commented 3 years ago

Yep, that looks like the right answer. Did you push it? … On 22/02/2021, zxxunkxzhx @.**> wrote: I write it as #define IS_MATCH ( (TRAN)1 << (8 sizeof(TRAN) - 1) ) #define IS_SUFFIX ( (TRAN)1 << (8 * sizeof(TRAN) - 2) ) #define T_FLAGS ( IS_MATCH | IS_SUFFIX ) and found it's okay -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #34 (comment) -- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

I did not push it, as I only simply verified on vs2019 64 bit configuration, not sure if this fix works for all cases.

thanks.

zxxunkxzhx commented 3 years ago

curious: why not static const TRAN T_FLAGS (IS_MATCH | IS_SUFFIX) ? … On 15/01/2021, changnet @.**> wrote: ```c typedef uint64_t TRAN, STATE, STRNO; enum { IS_MATCH = (TRAN)1 << (8sizeof(TRAN) - 1), IS_SUFFIX = (TRAN)1 << (8sizeof(TRAN) - 2), T_FLAGS = IS_MATCH | IS_SUFFIX }; Visual Studio(VS2013 VS2015 VS2019) warn about `warning C4309: 'initializing' : truncation of constant value` and actually truncate it，the value become zero. ![1](https://user-images.githubusercontent.com/9293777/104793540-131dac00-57de-11eb-8dac-748d9a248f54.jpg) in modern gcc(gcc 8 or gcc 10), if compile with `-pedantic` will given a warning. anyway, the value is expected.bash @ MSYS /tmp $ gcc -Wall -pedantic test.c test.c:6:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic] 6 | IS_MATCH = (TRAN)1 << (8sizeof(TRAN) - 1), | ^ test.c:7:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic] 7 | IS_SUFFIX = (TRAN)1 << (8sizeof(TRAN) - 2), | ^ test.c:8:17: 警告：ISO C restricts enumerator values to range of 'int' [-Wpedantic] 8 | T_FLAGS = IS_MATCH | IS_SUFFIX | ^~~~ @ MSYS /tmp $ ./a.exe Hello World 9223372036854775808 maybe using `static const` instead ?c static const TRAN IS_MATCH = (TRAN)1 << (8sizeof(TRAN) - 1); static const TRAN IS_SUFFIX = (TRAN)1 << (8sizeof(TRAN) - 2); static const TRAN T_FLAGS = ((TRAN)1 << (8sizeof(TRAN) - 1)) | ((TRAN)1 << (8*sizeof(TRAN) - 1)); ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #34 -- Engineers think equations approximate reality. Physicists think reality approximates the equations. Mathematicians never make the connection.

I don't care about other solutions, I just think

define T_FLAGS ( IS_MATCH | IS_SUFFIX )

maybe correct^^

changnet commented 3 years ago

static const TRAN T_FLAGS (IS_MATCH | IS_SUFFIX); using a constructor to initialize T_FLAGS, it only works at C++. static const TRAN T_FLAGS = IS_MATCH | IS_SUFFIX; works at modern complier, but not old c complier like vs2013, it hits a C2099 error

quote from stackoverflow

For example, this is NOT a constant

const int N = 5; /* `N` is not a constant in C */

The above N would be a constant in C++, but it is not a constant in C. So, if you try doing

static int j = N; /* ERROR */

you will get the same error: an attempt to initialize a static object with a non-constant