jimtcl
jimtcl copied to clipboard
MSVC compatibility fixes - mostly casts to avoid warnings about size …
…truncations - fully compiles on MS VC++ Express 2008
Includes generation of jimsh0 and conversion of tcl scripts to C Adds MSVC project files and configuration files (since configure script isn't needed/used)
I have used the static analyser in MSVC to find many issues (each category is in a separate commit)
I have separated out changes for MSVC support into simple - easy to understand commits. The largest one "Add explicit typecasts..." has no functionality changes - just typecasting to reduce difficulty of reviewing. Builds and runs cleanly using VisualStudio 2015 Community Edition.
(Fixed a minor issue which was causing some warnings in non-MSVC build)
Thanks. There are some things here I am happy to take, some things I am not, and some things that require further discussion. I'll work through them slowly as I have time, starting next week.
No problem - Let me know if you want me to change some of them when you're ready.
I'll update the pull request with the latest from my repo now so you're reviewing the latest stuff...
Whoops - wrong github account...
Also, I have been working on the "interp" command code - implementing it to work in the same way as standard TCL
Here are a couple of general comments:
- There is no point in submitting a patch to update autosetup/jimsh0.c since it is supplied by the autosetup project. It will only get updated when a new version of autosetup is imported
- We don't check alloc failures. We rely on the system aborting the process or otherwise handling alloc failures. The problem is that you will create a huge amount of code trying to do something reasonable as you percolate the failure back up through the system. The reality is that if you are out of memory, your application is toast. If your OS or libc doesn't do it, link against a local allocator that implements checking malloc/realloc.
- I don't mind very minor changes to keep static analysers happy, but I won't make wholesale changes that does nothing but avoid false positives. That is what exception lists are for.
- I'm really not happy with hundreds of type casts. The problem with type casting is that you can sweep real problems under the carpet. Casting should be for places where the compiler can't reasonably get it right. Why are so many casts needed to keep msvc happy?
- Please try hard to follow the STYLE guidelines. e.g. no tabs
- Will all your changes work on both 32 and 64 bit windows? e.g. GetTickCount64()
- Why do we need to add all this logging and heap checking to Jim Tcl? On Unix systems we have valgrind to do this. Is there no such option on Windows? I don't think this belongs here.
- Have you tested to ensure that cygwin and mingw32 still work with these changes? e.g. JIM_SPRINTF_DOUBLE_NEEDS_FIX
We don't check alloc failures
So you are happy to have the system cause seg-faults? I would have thought that the interpreter should bail out nicely so that whatever program it has been embedded into could handle shutdown nicely... (the user program may need to save data before exit)
Why do we need to add all this logging and heap checking to Jim Tcl?
So that the heap checking can become part of the automated tests. Also so that the debugger can take you directly to the allocation which is causing any corruption/leakage.
I'm really not happy with hundreds of type casts
These are virtually all legitimate warnings. Mostly due to :
- Assigning between signed & unsigned
- Assigning between wide & double
- Assigning between 32-bit values and values which can be 64-bit on a 64-bit machine
- Freeing variables declared as const**
I agree that using typecasts is not ideal - many of these probably indicate variables which have the wrong type.
Will all your changes work on both 32 and 64 bit windows?
Unfortunately I have no access to a 32-bit machine for testing, but the GetTickCount64() is not a 64-bit specific function - it just returns a 64-bit tick count in order to avoid the 32-bit rollover issues.
Please try hard to follow the STYLE guidelines.
I'll go back through the commits and check them for this.
Have you tested to ensure that cygwin and mingw32 still work with these changes?
I've tested with MSYS2 - I'll try the others.
I've updated these to be rebased on top of the changes needed to fix MinGW compilation.
I have moved the changes to autosetup/jimsh0.c into one temporary commit which I can abandon later.
I have also fixed style issues and tested on MSVC, MSYS2, MinGW32 and Cygwin.
I have taken most of the commits I'm obviously happy with. Since I can't test with MSVC, I am hesitant to take any large changes there.
Feel free to rebase to master and justify any further commits. I am very unlikely to take the alloc-related changes, heap debugging or logging changes.
Regarding allocation, see pull request #52 for a proposal