Linkage #defines are unused
(1) By David Matson (dmatson) on 2021-07-08 01:17:23 [link]
The amalgamation sqlite3.c has: ```c /* ** Provide the ability to override linkage features of the interface. */ #ifndef SQLITE_EXTERN # define SQLITE_EXTERN extern #endif #ifndef SQLITE_API # define SQLITE_API #endif #ifndef SQLITE_CDECL # define SQLITE_CDECL #endif #ifndef SQLITE_APICALL # define SQLITE_APICALL #endif #ifndef SQLITE_STDCALL # define SQLITE_STDCALL SQLITE_APICALL #endif #ifndef SQLITE_CALLBACK # define SQLITE_CALLBACK #endif #ifndef SQLITE_SYSAPI # define SQLITE_SYSAPI #endif ``` But most of these #defines are either unused or nearly unused. SQLITE_API is used extensively, but SQLITE_CDECL is only used 3 times, SQLITE_EXTERN is only used once, and SQLITE_APICALL, SQLITE_STDCALL, SQLITE_CALLBACK and SQLITE_SYSAPI are never used. Should these #defines be used? For example, should the last three parameters to sqlite3_create_function be marked as SQLITE_CALLBACK? (Or, if not, should these #defines be removed from the amalgamation?) Thanks, David
(2) By Larry Brasfield (larrybr) on 2021-07-08 01:36:33 in reply to 1 [link]
Those defines are not there for use by the SQLite library itself or its main client in the project known as the CLI shell. Those defines are there in case library users need to solve problems that would otherwise require more invasive trickery. For example, we have seen name collisions where more than 1 version/copy of the library is used in a single application image. Before you could make believable "SQLITE_xxx is used only # times" assertions, you would have to have obtained and analyzed a great number of client application sources. I dare to surmise that you have not done that, and never will. I do not mean to discourage code critiques with this.
(3) By David Matson (dmatson) on 2021-07-08 01:52:55 in reply to 2 [link]
Could you clarify how #defines not used by sqlite3.c help in those cases? For example, the only reference inside sqlite3.c to SQLITE_APICALL or SQLITE_SYSAPI are the lines pasted above. How does having these values #defined to empty help with client applications? (If they are unused by sqlite3.c, couldn't client applications create and use their own defines?) Thanks, David
(8) By Larry Brasfield (larrybr) on 2021-07-08 14:40:34 in reply to 3 [link]
As I mentioned, and as the comment, "Provide the ability to override linkage features of the <b>interface</b>.", (bolding added), suggests, those PP symbols are an outward facing feature rather than merely an implementation detail. There is extremely low likelihood that they will change or vanish for that reason. We would not like to force those library users who rely on them to have to find another solution, or (heaven forbid) be forced to munge the sources.
(4.1) By David Matson (dmatson) on 2021-07-08 02:40:26 edited from 4.0 in reply to 2 [link]
Also, to clarify, the statements above were made within the scope of the sqlite3.c file, where these lines were listed. Unless sqlite3.c is #included into another compilation unit (which seems unusual, though not impossible), I believe these defines in this file will have no effect, since they will only impact the compilation of this .c file (which doesn't use them). (And, even for the #include scenario, it seems like they have minimal usefulness, as the outer file can #define and then use whatever names it desires.) There may be other reasons to have them, but within the sqlite3.c file itself many of them appear to be unnecessary, perhaps vestigial.
(6.1) By Keith Medcalf (kmedcalf) on 2021-07-08 05:00:03 edited from 6.0 in reply to 4.1 [link]
See tool/mksqlite3c.tcl The full set of api annotations is not used unless you specify --apicall when running mksqlite3c.tcl. By default, only a subset of the calling convention qualifiers are used such as SQLITE_API and SQLITE_STATIC. This allows, for example, in the default case, the specification of -DSQLITE_API=__declspec(dllexport) when compiling sqlite3.c so that one does not need to "fiddle" with export definitions on platforms (such as Windows) that need to be told which symbols are to be exported from Discontiguous Saved Segments.
(10) By David Matson (dmatson) on 2021-07-08 23:50:44 in reply to 6.1 [link]
Thanks, Keith! That makes sense. Could/should the following defines be omitted from the amalgamation output when --apicall is not passed to mksqlite3c.tcl? * SQLITE_CALLBACK * SQLITE_SYSAPI * SQLITE_APICALL * SQLITE_STDCALL Otherwise, would the comment at the top saying that these defines can be used to override linkage be technically incorrect, at least with respect to the final amalgamation file produced? For SQLITE_STDCALL specifically, as far as I can tell, it looks like that one is no longer in use - compared to where it used to be in the [commit](https://github.com/sqlite/sqlite/commit/d69e5579400f93b27771324c08ad2c784371e995#diff-46262510279d071299462453bec166c33f04ec678224a5149c0420872f23e2f5R118) where it was first added, its use has now apparently been replaced entirely by [SQLITE_APICALL](https://github.com/sqlite/sqlite/blob/master/tool/mksqlite3c.tcl#L254). Is SQLITE_STDCALL vestigial now? Thanks, David
(11) By Larry Brasfield (larrybr) on 2021-07-09 00:42:34 in reply to 10 [link]
Now that we've gotten to "some of these macros are unused", (and due to your persistence, for which I credit you), I have looked at this enough to agree that there is some vestigial remnant of thinking that may have been apropos at one time but has become, at least for now, inapplicable. > \[W\]ould the comment at the top saying that these defines can be used to override linkage be technically incorrect ...? I would agree that if "these defines" means "each and every one of these defines", then the comment's assertion is incorrect. I'm not so sure (as you) that we should interpret comments in the code with a contract-writing lawyer's precision. I read the comment as "This set of defines is intended to allow so and so." However, being fond of precise locution myself, I would give you the point on this contention. But I must say that I am glad that SQLite's authors have spent their time more on the code than on trying to satisfy lawyerly readers. I am tempted to volunteer to clean this up, to the extent it needs cleanup. That extent is the crux. I'm working on something more useful and likely welcome at the moment. (Well, moments not spent writing posts.) We (SQLite developers) recognize that the docs can better approach perfection, and that some of the code can be improved for clarity and ease of maintenance. Among all such possible improvements, I have to say that this falls close to the bottom in my estimation. I mean no offense by this.
(12) By David Matson (dmatson) on 2021-07-09 01:53:10 in reply to 11 [link]
Thanks, Larry. In practice, when we compile winsqlite3.dll, we set all of these defines, even though it looks like they end up doing nothing - I think we looked at that comment and took everything it listed as being part of the interface. But we also have other copies of SQLite, which eventually should be merged, and the linkage is among the biggest difference. Understanding what actually matters vs. what doesn't can really help (especially when it's in the middle of a very complex system). We also have many other places where we list both SQLITE_API and SQLITE_STDCALL on the function signature of alternately-compiled stubs - cleaning this all up would be easier if it's clearer what these defines are and do (and if there are fewer to manage). It would simplify things enough for me that I'd be willing to offer contributing a patch to clean this up, if that would help. I'm not sure if you have a contribution process though, or in this case if it would take less work on your side to do it without my involvement. Let me know what you think. Thanks, David
(13) By Larry Brasfield (larrybr) on 2021-07-09 13:02:43 in reply to 12
> In practice, when we compile winsqlite3.dll, we set all of these defines, even though it looks like they end up doing nothing - I think we looked at that comment and took everything it listed as being part of the interface. A prudent choice. We should generally rely on published interfaces rather than reasoning about the interior of the black or dark-gray box within. I consider it an interface deficiency when one must look within rather than at the docs or doc-like comments. > But we also have other copies of SQLite, which eventually should be merged, and the linkage is among the biggest difference. Understanding what actually matters vs. what doesn't can really help (especially when it's in the middle of a very complex system). Hmmm. That eventual merge seems like a very strange feature for a library, well tested and with a well documented, published API, to try to support. I can only guess as to how that should affect API design or evolution. My reaction to this (from this project's viewpoint) is that any genuine need to delve into our gray box bespeaks an opportunity to correct a documentation deficiency. > We also have many other places where we list both SQLITE_API and SQLITE_STDCALL on the function signature of alternately-compiled stubs - cleaning this all up would be easier if it's clearer what these defines are and do ... The docs should carry the burden on that clarity. I think a minimum outcome of this discussion will be some clear documentation on those "linkage" (and calling convention) details. > ... (and if there are fewer to manage). There lies an interesting issue. I intend to come back to this after some consultation. > It would simplify things enough for me that I'd be willing to offer contributing a patch to clean this up, if that would help. I'm not sure if you have a contribution process though, or in this case if it would take less work on your side to do it without my involvement. Let me know what you think. The project does not directly incorporate patches except from people who have signed a contributor's agreement serving to protect the codebase's unlicensed nature. Suggestions are welcome. However, it is not yet clear what fraction of the presently "unused"<sup><b>1</b></sup> should be reserved for future use as the API evolves. So it would be premature to begin implementing a change. As you might perceive, SQLite API design and build provisions are done and managed very carefully by Richard. > Thanks, Thanks for paying attention to possible quality issues. ---- <b>1.</b> We will have to consider which unused interface macros should be presently used, and what to do about any such.
(14) By Larry Brasfield (larrybr) on 2021-07-09 20:10:04 in reply to 12 [link]
(Following up on my post #13:) Upon review, it appears that all of those linkage and calling convention marker macros are used at one time or another.<sup>1</sup> So the only remaining action that I see is to make the comment describing them more precise. Because the amalgamation is generated, it may make sense to suppress the appearance of those macros which are unused in a particular generated form of the code, or say "reserved" for the ones not used in the file following. I do agree that the significance of each one should be documented so as to not impose needless work, concern, or effort searching code to discern meaning. I presume such a resolution would satisfy your inquiry on this subject. ---- 1. In addition to the amalgamation dropped for releases, the project support several variations of its generation process. So the "published" (dropped) amalgamation does not tell the whole story on those interface macro.
(15) By David Matson (dmatson) on 2021-07-09 20:34:02 in reply to 14 [link]
Larry, Yes, if the amalgamation could omit (or mark as reserved) any macros it doesn't happen to use, that would be excellent. If they're still present in some form, having the amalgamation indicate which are actually used and which aren't would be nice. Any documentation or comments on how to use these macros correctly would also be appreciated. After looking at the --apicall generation option Keith mentioned above, the only one left that I couldn't find used at all was SQLITE_STDCALL. (Specifically, it didn't show up in a search on the GitHub mirror repo, but that may not have found all cases.) But if it's omitted from the amalgamation, it won't make much difference to me whether SQLITE_STDCALL is still in the repo internally for some other case. Thanks, David
(16) By Keith Medcalf (kmedcalf) on 2021-07-09 21:24:41 in reply to 15 [link]
I believe the idea behind SQLITE_STDCALL and SQLITE_CDECL is that those macro's can be defined to how the specific compiler annotates those calling conventions, and then those define's in turn may be used in by the SQLITE_APICALL, SQLITE_CALLBACK, etc., defines. The whole point being to allow the complete definition of non-standard calling sequences as might be used by some Operating Systems (such as Windows) that use the Pascal calling sequence rather than the default cdecl calling sequences. If you do not use --apicall when running mksqlite3c.tcl and mksqlite3h.tcl then the sqlite3.c and sqlite3.h files are output with minimal annotations designed for use where you **do not** need to override the default calling conventions used by the compiler. Using --apicall when running mksqlite3c.tcl and mksqlite3h.tcl however litters the generated code with these classifiers so that you can, if you choose to do so, cohesively control the calling conventions used in the generation of the object and load modules -- so for example you can make the sqlite3 module use Windows calling conventions and look just like any old standard Windows API (which would presumably also allow you to create 32 to 64 bit trampolines if you chose to do so to allow "sqlite3" to be a "system component".)
(7) By Keith Medcalf (kmedcalf) on 2021-07-08 05:11:38 in reply to 4.1 [link]
The can also be used as, for example, in APSW so that the sqlite3.c module can be #include into the wrapper source code with `#define SQLITE_API static` so that the sqlite3 api symbols become module locals and the code can be optimized by the compiler without reference to maintenance of entry point conventions for the api.
(9) By Larry Brasfield (larrybr) on 2021-07-08 14:44:31 in reply to 4.1 [link]
> Unless sqlite3.c is #included into another compilation unit (which seems unusual, though not impossible), I believe these defines in this file will have no effect, ... That's where you are completely overlooking a critical fact. To see what that is, I suggest you override some/all of those PP definitions, and compile to a .o (or .obj) and see whether the after object matches the before object. (They differ for most such overrides.)
(5) By mistachkin on 2021-07-08 03:30:57 in reply to 1 [link]
These defines are all used by various build processes, some of which are internal.