BUG: Illegal initialization in icu.c : sqlite3IcuInit

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
44 messages Options
123
Reply | Threaded
Open this post in threaded view
|

BUG: Illegal initialization in icu.c : sqlite3IcuInit

Ziemowit Laski
Visual C++ correctly catches this.  The fragment

  struct IcuScalar {
    const char *zName;                        /* Function name */
    int nArg;                                 /* Number of arguments */
    int enc;                                  /* Optimal text encoding */
    void *pContext;                           /* sqlite3_user_data() context */
    void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
  } scalars[] = {
    {"regexp", 2, SQLITE_ANY|SQLITE_DETERMINISTIC,          0, icuRegexpFunc},

    {"lower",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
    {"lower",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
    {"upper",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},
    {"upper",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},

    {"lower",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
    {"lower",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
    {"upper",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},
    {"upper",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},

    {"like",   2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},
    {"like",   3, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},

    {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
  };

  int rc = SQLITE_OK;
  int i;

should read

       struct IcuScalar {
              const char *zName;                        /* Function name */
              int nArg;                                 /* Number of arguments */
              int enc;                                  /* Optimal text encoding */
              void *pContext;                           /* sqlite3_user_data() context */
              void(*xFunc)(sqlite3_context*, int, sqlite3_value**);
       } scalars[] = {
              { "regexp", 2, SQLITE_ANY | SQLITE_DETERMINISTIC,          0, icuRegexpFunc },

              { "lower",  1, SQLITE_UTF16 | SQLITE_DETERMINISTIC,        0, icuCaseFunc16 },
              { "lower",  2, SQLITE_UTF16 | SQLITE_DETERMINISTIC,        0, icuCaseFunc16 },
              { "upper",  1, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 },
              { "upper",  2, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 },

              { "lower",  1, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuCaseFunc16 },
              { "lower",  2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuCaseFunc16 },
              { "upper",  1, SQLITE_UTF8 | SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16 },
              { "upper",  2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16 },

              { "like",   2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuLikeFunc },
              { "like",   3, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuLikeFunc },

              { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
       };

       int rc = SQLITE_OK;
       int i;

       scalars[11].pContext = (void*)db;

Thank you,

--Zem
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Clemens Ladisch
Ziemowit Laski wrote:
> Visual C++

Which one?

> correctly catches this.

Oh?  What exactly is illegal about this?

>   struct IcuScalar {
>     const char *zName;                        /* Function name */
>     int nArg;                                 /* Number of arguments */
>     int enc;                                  /* Optimal text encoding */
>     void *pContext;                           /* sqlite3_user_data() context */
>     void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>   } scalars[] = {
>     ...
>     {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>   };
>
> should read
>
>        } scalars[] = {
>               ...
>               { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
>        };
>
>        scalars[11].pContext = (void*)db;


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Hick Gunter
On ILP_32 architectures, the integer 0 is not discernible from the (void *)0 (aka NULL) and so most compilers don't bother to issue a warning. This comes from an age where programmers were expected to know how computers work on an assembly language level and would "know what they are doing" when passing an integer of the same size to a pointer argument.

On LP_64 architactures, the integer 0 is 32 bits while (void *)0 is 64 bits, which makes more than a bit of a difference. A 64 bit integer 0 would be denoted by 0L. Additionally, compilers have become more pedantic (because compiler builders have become aware of the fact that not "knowing what you are doing" in respect to pointers and integers is a common source of errors) and will complain about "making a pointer from an integer of a different size" and/or "making a pointer from an integer without a cast".

There are many casese of 0 vs. NULL in sqlite code; they do not, however, influence the operation of the code, just compileability in "nit picking" mode.

-----Ursprüngliche Nachricht-----
Von: sqlite-users [mailto:[hidden email]] Im Auftrag von Clemens Ladisch
Gesendet: Donnerstag, 26. Jänner 2017 08:47
An: [hidden email]
Betreff: Re: [sqlite] BUG: Illegal initialization in icu.c : sqlite3IcuInit

Ziemowit Laski wrote:
> Visual C++

Which one?

> correctly catches this.

Oh?  What exactly is illegal about this?

>   struct IcuScalar {
>     const char *zName;                        /* Function name */
>     int nArg;                                 /* Number of arguments */
>     int enc;                                  /* Optimal text encoding */
>     void *pContext;                           /* sqlite3_user_data() context */
>     void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>   } scalars[] = {
>     ...
>     {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>   };
>
> should read
>
>        } scalars[] = {
>               ...
>               { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
>        };
>
>        scalars[11].pContext = (void*)db;


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users


___________________________________________
 Gunter Hick
Software Engineer
Scientific Games International GmbH
FN 157284 a, HG Wien
Klitschgasse 2-4, A-1130 Vienna, Austria
Tel: +43 1 80100 0
E-Mail: [hidden email]

This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is confidential, privileged or legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Clemens Ladisch
Hick Gunter wrote:
> On ILP_32 architectures, the integer 0

What integer 0?  The message is about initializing scalars[11].pContent
(a "void*") with "(void*)db", which is "sqlite3*".

> Oh?  What exactly is illegal about this?
>
>>   struct IcuScalar {
>>     const char *zName;                        /* Function name */
>>     int nArg;                                 /* Number of arguments */
>>     int enc;                                  /* Optimal text encoding */
>>     void *pContext;                           /* sqlite3_user_data() context */
>>     void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>>   } scalars[] = {
>>     ...
>>     {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>   };
>>
>> should read
>>
>>        } scalars[] = {
>>               ...
>>               { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
>>        };
>>
>>        scalars[11].pContext = (void*)db;


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Hick Gunter
The OP proposes intializing the structure member with "0" instead of "(void*)db", which I read the other way around and reminds me of certain implicit equivalences 0 <==> (void*)0, on eof which was recently discussed here.

Perhaps the OP's Compiler does not allow initialization of a dynamic structure with a parameter value.

-----Ursprüngliche Nachricht-----
Von: sqlite-users [mailto:[hidden email]] Im Auftrag von Clemens Ladisch
Gesendet: Donnerstag, 26. Jänner 2017 13:07
An: [hidden email]
Betreff: Re: [sqlite] BUG: Illegal initialization in icu.c : sqlite3IcuInit

Hick Gunter wrote:
> On ILP_32 architectures, the integer 0

What integer 0?  The message is about initializing scalars[11].pContent (a "void*") with "(void*)db", which is "sqlite3*".

> Oh?  What exactly is illegal about this?
>
>>   struct IcuScalar {
>>     const char *zName;                        /* Function name */
>>     int nArg;                                 /* Number of arguments */
>>     int enc;                                  /* Optimal text encoding */
>>     void *pContext;                           /* sqlite3_user_data() context */
>>     void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>>   } scalars[] = {
>>     ...
>>     {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>   };
>>
>> should read
>>
>>        } scalars[] = {
>>               ...

Look right below here...

>>               { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
>>        };
>>

Look just above here...

>>        scalars[11].pContext = (void*)db;


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users


___________________________________________
 Gunter Hick
Software Engineer
Scientific Games International GmbH
FN 157284 a, HG Wien
Klitschgasse 2-4, A-1130 Vienna, Austria
Tel: +43 1 80100 0
E-Mail: [hidden email]

This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is confidential, privileged or legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

dandl
In reply to this post by Ziemowit Laski

>>>From: sqlite-users [mailto:[hidden email]] On Behalf Of Ziemowit Laski
Sent: Thursday, 26 January 2017 7:36 AM
To: [hidden email]
Subject: [sqlite] BUG: Illegal initialization in icu.c : sqlite3IcuInit

>>>
>>>Visual C++ correctly catches this.  The fragment
>>>
>>>  struct IcuScalar {
>>>    const char *zName;                        /* Function name */
>>>    int nArg;                                 /* Number of arguments */
>>>    int enc;                                  /* Optimal text encoding */
>>>    void *pContext;                           /* sqlite3_user_data() context */
>>>    void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>>>  } scalars[] = {
>>>    {"regexp", 2, SQLITE_ANY|SQLITE_DETERMINISTIC,          0, icuRegexpFunc},
>>>
>>>    {"lower",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
>>>    {"lower",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
>>>    {"upper",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},
>>>    {"upper",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},
>>>
>>>    {"lower",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
>>>    {"lower",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
>>>    {"upper",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},
>>>    {"upper",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},
>>>
>>>    {"like",   2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},
>>>    {"like",   3, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},
>>>
>>>    {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>>  };
>>>
>>>  int rc = SQLITE_OK;
>>>  int i;
>>>
>>>should read
>>>
>>>       struct IcuScalar {
>>>              const char *zName;                        /* Function name */
>>>              int nArg;                                 /* Number of arguments */
>>>              int enc;                                  /* Optimal text encoding */
>>>              void *pContext;                           /* sqlite3_user_data() context */
>>>              void(*xFunc)(sqlite3_context*, int, sqlite3_value**);
>>>       } scalars[] = {
>>>              { "regexp", 2, SQLITE_ANY | SQLITE_DETERMINISTIC,          0, icuRegexpFunc },
>>>
>>>              { "lower",  1, SQLITE_UTF16 | SQLITE_DETERMINISTIC,        0, icuCaseFunc16 },
>>>              { "lower",  2, SQLITE_UTF16 | SQLITE_DETERMINISTIC,        0, icuCaseFunc16 },
>>>              { "upper",  1, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 },
>>>              { "upper",  2, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 },
>>>
>>>              { "lower",  1, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuCaseFunc16 },
>>>              { "lower",  2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuCaseFunc16 },
>>>              { "upper",  1, SQLITE_UTF8 | SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16 },
>>>              { "upper",  2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16 },
>>>
>>>              { "like",   2, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuLikeFunc },
>>>              { "like",   3, SQLITE_UTF8 | SQLITE_DETERMINISTIC,         0, icuLikeFunc },
>>>
>>>              { "icu_load_collation",  2, SQLITE_UTF8, 0, icuLoadCollation }
>>>       };
>>>
>>>       int rc = SQLITE_OK;
>>>       int i;
>>>
>>>       scalars[11].pContext = (void*)db;
>>>

Why would you say that? What error message are you getting on what compiler, and why would that change produce any different result? What type is 'db'?

Can you quote some specified reference to the C standard document in support of your contention?

Regards
David M Bennett FACS

Andl - A New Database Language - andl.org





_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Richard Damon
In reply to this post by Hick Gunter
On 1/26/17 8:05 AM, Hick Gunter wrote:
> The OP proposes intializing the structure member with "0" instead of "(void*)db", which I read the other way around and reminds me of certain implicit equivalences 0 <==> (void*)0, on eof which was recently discussed here.
>
> Perhaps the OP's Compiler does not allow initialization of a dynamic structure with a parameter value.
>
If the structure is a static/global, then the initializers need to be
compile time constants for C. If this is a function local object (which
it looks like), that initialization with a variable is perfectly legal
(but perhaps VC is warning that the presence of the variable is going to
make that initilization less efficient, as it can't just make a static
copy of the initialization value and memcpy that to the variable).


--
Richard Damon

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

David Empson
In reply to this post by Clemens Ladisch

> On 26/01/2017, at 8:46 PM, Clemens Ladisch <[hidden email]> wrote:
>
> Ziemowit Laski wrote:
>> Visual C++
>
> Which one?
>
>> correctly catches this.
>
> Oh?  What exactly is illegal about this?
>
>>  struct IcuScalar {
>>    const char *zName;                        /* Function name */
>>    int nArg;                                 /* Number of arguments */
>>    int enc;                                  /* Optimal text encoding */
>>    void *pContext;                           /* sqlite3_user_data() context */
>>    void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>>  } scalars[] = {
>>    ...
>>    {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>  };

The ANSI/ISO C 1990 standard states this in section 6.5.7, under Constraints:

“All the expressions in an initializer for an object that has static storage duration or in an initializer list for an object that has aggregate or union type shall be constant expressions.”

In this case the code is trying to initialize a field of an auto struct using the db parameter passed to the function. That is not a constant expression, and it is in an initializer list for an object that has aggregate type (whether or not the object has static storage duration), so is disallowed under ANSI/ISO C 1990.

Later versions of the C standard removed the bit about aggregate or union types, leaving only the static restriction, e.g. from section 6.7.8 of the draft C99 standard:

"All the expressions in an initializer for an object that has static storage duration shall be constant expressions or string literals.”

Visual C++ is based on C90, and assuming Wikipedia has the details right, it wasn’t until Visual C++ 2013 that Microsoft started making changes to support C99.

Should SQLite be aiming for the 1990 version of ANSI/ISO C as a baseline, for widest compatibility, or is it OK to drop older compilers and require C99 compliance?

The only obvious reference I found in the SQLite documentation was http://www.sqlite.org/howtocompile.html which mentions “ANSI-C”. That is generally understood to mean the ANSI C 1989 standard, which was adopted internationally as ISO 9899:1990.

--
David Empson
[hidden email]

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

dandl
From: sqlite-users [mailto:[hidden email]] On Behalf Of David Empson

>>>The ANSI/ISO C 1990 standard states this in section 6.5.7, under Constraints:
>>>“All the expressions in an initializer for an object that has static storage duration or in an initializer list for an object that has aggregate or union type shall be constant expressions.”

>>>In this case the code is trying to initialize a field of an auto struct using the db parameter passed to the function. That is not a constant expression, and it is in an initializer list for an object that has aggregate type (whether or not the object has static storage duration), so is disallowed under ANSI/ISO C 1990.

>>>Later versions of the C standard removed the bit about aggregate or union types, leaving only the static restriction, e.g. from section 6.7.8 of the draft C99 standard:
>>>"All the expressions in an initializer for an object that has static storage duration shall be constant expressions or string literals.”

I can confirm that. So in summary, the Sqlite code is not valid ANSI C (1990) but it is valid according to the C99 standard. It's only broken for old compilers, not new ones.

1. Why was VS the first compiler to detect this?
2. Is there an authoritative view on which standard Sqlite should comply with?

Regards
David M Bennett FACS

Andl - A New Database Language - andl.org





_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Richard Hipp-3
On 1/26/17, dandl <[hidden email]> wrote:
>
> 1. Why was VS the first compiler to detect this?

Perhaps VS is the only compiler still in frequent use that does not support C99?

> 2. Is there an authoritative view on which standard Sqlite should comply
> with?
>

SQLite should compile with VS.  I checked in a fix for the problem
here: https://www.sqlite.org/src/info/50e60cb44fd3687d

--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Dominique Devienne
On Fri, Jan 27, 2017 at 1:24 AM, Richard Hipp <[hidden email]> wrote:

> On 1/26/17, dandl <[hidden email]> wrote:
> >
> > 1. Why was VS the first compiler to detect this?
>
> Perhaps VS is the only compiler still in frequent use that does not
> support C99?


I guess David's question can also be rephrased as to why didn't the SQLite
team's
internal CI detect this before the community did?

SQLite is known for its outstanding testing in general, so I guess it's
surprising that if it
targets ANSI C 1989/1990 compatibility there isn't a build-bot that checks
this continuously. --DD
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Kim Gräsman
In reply to this post by David Empson
On Thu, Jan 26, 2017 at 10:08 PM, David Empson <[hidden email]> wrote:

>
>> On 26/01/2017, at 8:46 PM, Clemens Ladisch <[hidden email]> wrote:
>>
>> Ziemowit Laski wrote:
>>> Visual C++
>>
>> Which one?
>>
>>> correctly catches this.
>>
>> Oh?  What exactly is illegal about this?
>>
>>>  struct IcuScalar {
>>>    const char *zName;                        /* Function name */
>>>    int nArg;                                 /* Number of arguments */
>>>    int enc;                                  /* Optimal text encoding */
>>>    void *pContext;                           /* sqlite3_user_data() context */
>>>    void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
>>>  } scalars[] = {
>>>    ...
>>>    {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>>  };
>
> The ANSI/ISO C 1990 standard states this in section 6.5.7, under Constraints:
>
> “All the expressions in an initializer for an object that has static storage duration or in an initializer list for an object that has aggregate or union type shall be constant expressions.”

But this object doesn't have static storage duration, does it?
`scalars` is just a local variable in a function:
https://sourcecodebrowser.com/sqlite3/3.6.21/icu_8c_source.html#l00449

unless I'm looking at the wrong version.

Again, it would be nice to see the actual warning from MSVC.

FWIW,
- Kim
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

David Empson

> On 27/01/2017, at 9:09 PM, Kim Gräsman <[hidden email]> wrote:
>
> On Thu, Jan 26, 2017 at 10:08 PM, David Empson <[hidden email] <mailto:[hidden email]>> wrote:
>>
>>> On 26/01/2017, at 8:46 PM, Clemens Ladisch <[hidden email]> wrote:
>>>
>>>> …
>>>>  {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>>> };
>>
>> The ANSI/ISO C 1990 standard states this in section 6.5.7, under Constraints:
>>
>> “All the expressions in an initializer for an object that has static storage duration or in an initializer list for an object that has aggregate or union type shall be constant expressions.”
>
> But this object doesn't have static storage duration, does it?
> `scalars` is just a local variable in a function:
> https://sourcecodebrowser.com/sqlite3/3.6.21/icu_8c_source.html#l00449 <https://sourcecodebrowser.com/sqlite3/3.6.21/icu_8c_source.html#l00449>
>
> unless I'm looking at the wrong version.

That version does have the text quoted above.

The problem is that ANSI/ISO C 1990 stipulates that an initializer for any object of aggregate type, whether or not it is static, must be constant.

It doesn’t matter that this variable has auto storage - it is an aggregate type (array of structs), and the initializer for an aggregate type must be constant if a compiler is enforcing ANSI/ISO C 1990 rules (or is based on C90 but hasn’t implemented extensions to allow a non-constant initializer for an aggregate type).

As I noted, C99 changed the rules and does allow a non-constant initializer in this case.

The code in question has been there since May 2007, so the fact that nobody reported it until now suggests that almost nobody who uses the ICU extension is doing so with a strict ANSI C90 compiler (or a compiler set to enforce C90 rules).

Perhaps SQLite’s test procedure should be enforcing strict ANSI C mode? If this is already being done, then the compiler(s) used might not be enforcing this particular rule.

> Again, it would be nice to see the actual warning from MSVC.

Indeed.

As it happens, I have an installation of Visual C++ 2008 so can test this, not using icu.c as I don’t have necessary support files installed, but I can create a mockup using a copy of the sqlite3IcuInit function with stubs for everything it references.

Using default options from the command line, my test file compiles without error.

Therefore Visual C++ 2008 does have extensions to ANSI C90 which allow a non-constant initializer for an aggregate type with auto storage. (I also tested Visual C++ 98 and it worked there too.)

However if I tell Visual C++ to enable strict ANSI C mode by adding the /Za command line option, I get an error:


[C:\p\test]cl /Za test.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.c
test.c(45) : error C2097: illegal initialization


This is consistent with the rule I noted from ANSI/ISO C 1990.

For reference, here is my test.c:


typedef struct { int a; } sqlite3;
typedef struct { int b; } sqlite3_context;
typedef struct { int c; } sqlite3_value;

#define SQLITE_ANY 1
#define SQLITE_DETERMINISTIC 2
#define SQLITE_UTF16 4
#define SQLITE_UTF8 8

#define SQLITE_OK 0

void icuRegexpFunc(sqlite3_context*c, int i, sqlite3_value**vpp) { }
void icuCaseFunc16(sqlite3_context*c, int i, sqlite3_value**vpp) { }
void icuLikeFunc(sqlite3_context*c, int i, sqlite3_value**vpp) { }
void icuLoadCollation(sqlite3_context*c, int i, sqlite3_value**vpp) { }

int sqlite3_create_function(sqlite3 *db, const char *zName, int nArg, int enc, void *pContext,
                          void (*xFunc)(sqlite3_context*,int,sqlite3_value**), int i, int j) {
  return SQLITE_OK;
}

int sqlite3IcuInit(sqlite3 *db){
  struct IcuScalar {
    const char *zName;                        /* Function name */
    int nArg;                                 /* Number of arguments */
    int enc;                                  /* Optimal text encoding */
    void *pContext;                           /* sqlite3_user_data() context */
    void (*xFunc)(sqlite3_context*,int,sqlite3_value**);
  } scalars[] = {
    {"regexp", 2, SQLITE_ANY|SQLITE_DETERMINISTIC,          0, icuRegexpFunc},

    {"lower",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
    {"lower",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC,        0, icuCaseFunc16},
    {"upper",  1, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},
    {"upper",  2, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16},

    {"lower",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
    {"lower",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuCaseFunc16},
    {"upper",  1, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},
    {"upper",  2, SQLITE_UTF8|SQLITE_DETERMINISTIC,  (void*)1, icuCaseFunc16},

    {"like",   2, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},
    {"like",   3, SQLITE_UTF8|SQLITE_DETERMINISTIC,         0, icuLikeFunc},

    {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
  };

  int rc = SQLITE_OK;
  int i;

  for(i=0; rc==SQLITE_OK && i<(int)(sizeof(scalars)/sizeof(scalars[0])); i++){
    struct IcuScalar *p = &scalars[i];
    rc = sqlite3_create_function(
        db, p->zName, p->nArg, p->enc, p->pContext, p->xFunc, 0, 0
    );
  }

  return rc;
}

int main(void) {
  sqlite3 db;
  return sqlite3IcuInit(&db);
}


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Clemens Ladisch
David Empson wrote:
> Perhaps SQLite’s test procedure should be enforcing strict ANSI C mode?
> If this is already being done, then the compiler(s) used might not be
> enforcing this particular rule.

"gcc -pedantic -std=c90" (or gnu90) would check for this error:

test.c:6:3: warning: initializer element is not computable at load time [-Wpedantic]

> [...]
> Therefore Visual C++ 2008 does have extensions to ANSI C90 which allow
> a non-constant initializer for an aggregate type with auto storage.
> (I also tested Visual C++ 98 and it worked there too.)
>
> However if I tell Visual C++ to enable strict ANSI C mode by adding
> the /Za command line option, I get an error:
>
> test.c(45) : error C2097: illegal initialization

So neither compiler complains unless you explicitly tell it to.
(But that extra checking _is_ useful for compatibility.)


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Kim Gräsman
In reply to this post by David Empson
David,

On Fri, Jan 27, 2017 at 12:51 PM, David Empson <[hidden email]> wrote:

>
>> On 27/01/2017, at 9:09 PM, Kim Gräsman <[hidden email]> wrote:
>>
>> On Thu, Jan 26, 2017 at 10:08 PM, David Empson <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>>> On 26/01/2017, at 8:46 PM, Clemens Ladisch <[hidden email]> wrote:
>>>>
>>>>> …
>>>>>  {"icu_load_collation",  2, SQLITE_UTF8, (void*)db, icuLoadCollation},
>>>>> };
>>>
>>> The ANSI/ISO C 1990 standard states this in section 6.5.7, under Constraints:
>>>
>>> “All the expressions in an initializer for an object that has static storage duration or in an initializer list for an object that has aggregate or union type shall be constant expressions.”
>>
>> But this object doesn't have static storage duration, does it?
>> `scalars` is just a local variable in a function:
>> https://sourcecodebrowser.com/sqlite3/3.6.21/icu_8c_source.html#l00449 <https://sourcecodebrowser.com/sqlite3/3.6.21/icu_8c_source.html#l00449>
>>
>> unless I'm looking at the wrong version.
>
> That version does have the text quoted above.
>
> The problem is that ANSI/ISO C 1990 stipulates that an initializer for any object of aggregate type, whether or not it is static, must be constant.

Ah, misread the part about aggregate types in general. Sorry about the noise!

- Kim
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Clemens Ladisch
In reply to this post by Clemens Ladisch
I wrote:
> David Empson wrote:
>> Perhaps SQLite’s test procedure should be enforcing strict ANSI C mode?
>> If this is already being done, then the compiler(s) used might not be
>> enforcing this particular rule.
>
> "gcc -pedantic -std=c90" (or gnu90) would check for this error

And tool/warnings.sh already uses it, but not for the Android configuration,
which is the only one that enables ICU.


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

James K. Lowden
In reply to this post by Hick Gunter
On Thu, 26 Jan 2017 08:19:02 +0000
Hick Gunter <[hidden email]> wrote:

> On LP_64 architactures, the integer 0 is 32 bits while (void *)0 is
> 64 bits, which makes more than a bit of a difference. A 64 bit
> integer 0 would be denoted by 0L.

in C, as you know, integer assignment is subject to integer promotion.
If the LHS is wider, the RHS is widened to match.  The specification is
much more precise, of course, but that's the effect.  

There's nothing invalid of ambiguous about:

        long L = '\0';

It is the same as

        long L = 0L;

The same is true for pointers.  

--jkl
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

James K. Lowden
In reply to this post by David Empson
On Fri, 27 Jan 2017 10:08:17 +1300
David Empson <[hidden email]> wrote:

> Should SQLite be aiming for the 1990 version of ANSI/ISO C as a
> baseline, for widest compatibility, or is it OK to drop older
> compilers and require C99 compliance?

Older compilers?  C99 was adopted 17 years ago, and most of the changes
it standardardized had by then already been implemented by more than one
compiler.  

Sticking with C90 is perfectly rational if you're still running
Windows 98 on a Pentium III at 500 Mhz with 256 MB RAM.  Else, really,
it's not too soon to adopt a 6-year old standard, C11.  

--jkl


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Keith Medcalf
In reply to this post by Clemens Ladisch


On Friday, 27 January, 2017 08:26, Clemens Ladisch <[hidden email]> said:

> David Empson wrote:
> > Perhaps SQLite’s test procedure should be enforcing strict ANSI C mode?
> > If this is already being done, then the compiler(s) used might not be
> > enforcing this particular rule.
>
> "gcc -pedantic -std=c90" (or gnu90) would check for this error:
>
> test.c:6:3: warning: initializer element is not computable at load time [-
> Wpedantic]

But the error message/warning is of high bogosity.

While it is true that the initializer element cannot be computed at "load" time, it is not a "load time" item that is being initialized.  The item being initialized is a dynamic element on the stack inside a function call and it is initialized from a function argument.  It is entirely irrelevant that the element cannot be computed at "load time" because computing it thusly would be of no use whatsoever -- initialization of items residing on a functions call stack must always be done at "run time" and can never be done at "load time".

So the erroneous warning should just be ignored and a bug report filed against the compiler.




_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: BUG: Illegal initialization in icu.c : sqlite3IcuInit

Richard Damon
On 1/27/17 6:06 PM, Keith Medcalf wrote:

>
> On Friday, 27 January, 2017 08:26, Clemens Ladisch <[hidden email]> said:
>
>> David Empson wrote:
>>> Perhaps SQLite’s test procedure should be enforcing strict ANSI C mode?
>>> If this is already being done, then the compiler(s) used might not be
>>> enforcing this particular rule.
>> "gcc -pedantic -std=c90" (or gnu90) would check for this error:
>>
>> test.c:6:3: warning: initializer element is not computable at load time [-
>> Wpedantic]
> But the error message/warning is of high bogosity.
>
> While it is true that the initializer element cannot be computed at "load" time, it is not a "load time" item that is being initialized.  The item being initialized is a dynamic element on the stack inside a function call and it is initialized from a function argument.  It is entirely irrelevant that the element cannot be computed at "load time" because computing it thusly would be of no use whatsoever -- initialization of items residing on a functions call stack must always be done at "run time" and can never be done at "load time".
>
> So the erroneous warning should just be ignored and a bug report filed against the compiler.
>
except that the C90 standard apparently says that aggregates (like
this), need to be initialized to a constant (load time), possibly to
allow the function to initialize the variable with a memcpy (or
equivalent) from a constant initialization buffer. C99 apparently
decided that rather than making the programmer make the transformation
to update a few 'call time' values, it was better to make the compiler
do that work (as it would be less error prone).

--
Richard Damon

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
123