-
Notifications
You must be signed in to change notification settings - Fork 124
SIMH: Fix sim_printf_fmts.h for Visual Stduio 2013 and older #474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Per Microsoft documentation versions Visual Studio 2013 and below do not support the hh, j, z, and t length prefixes for printf format-type specifiers. Further, the preprocessor selection logic was modified so that 64-bit Visual Studio versions are actually recognized.
Just an aside comment outside of the actual commit message: I actually found the error because attempting to mount the IBM 1401 software kit tapes (e.g. AUTOCODER, FORTRAN, and the diagnostics tape) resulted in SIMH bailing when trying to print the "%SIM-INFO: contains bytes of tape data ( records, tapemarks)" message, but not when SIMH was set to quiet mode. Tracing the error showed the problem was the |
@cmgauger: Why the ancient compiler? The de facto C dialect is C99, if you believe the |
@cmgauger : Need to rethink this PR a bit so that it uses _MSC_VER more effectively because MS has become more standard compliant so that the "z" qualifier for Need to research when MS introduced the "z" qualifier and conditionalize the preprocessor statements accordingly, i.e., |
It appears that VS 2015 is when it actually became standards compliant. In any case, Microsoft's documentation from VS 2015 up to VS 2022 shows that
The documentation has that exact same paragraph even in VS 2022 documentation. Given that then, it seems that builds on VS should use
Which is funny, because the original, unmodified, set of conditionals still defaults to using #if defined (_WIN32) || defined(_WIN64)
# if defined(__MINGW64__)
# define LL_FMT "I64"
# define SIZE_T_FMT "I64"
# elif defined(_MSC_VER) || defined(__MINGW32__)
# define LL_FMT "ll"
# define SIZE_T_FMT "z"
# else
/* Graceful fail -- shouldn't ever default to this on a Windows platform. */
# define LL_FMT "ll"
# define SIZE_T_FMT "I32"
# endif
# define T_UINT64_FMT "I64"
# define T_INT64_FMT "I64"
# define POINTER_FMT "p"
So if we have to have MinGW use standards compliant size prefixes instead of Microsoft's own, it'll need to be changed to something like: #if defined (_WIN32) || defined(_WIN64)
# if defined(__MINGW64__) || defined(__MINGW32__)
/* MinGW, 32- or 64- bit: use standards compliant prefixes */
# define SIZE_T_FMT "z"
# define T_UINT64_FMT "j"
# define T_INT64_FMT "j"
# elif defined (_WIN64)
/* VS, 64-bit: use Microsoft's preferred I prefix */
# define SIZE_T_FMT "I64"
# define T_UINT64_FMT "I64"
# define T_INT64_FMT "I64"
# else
/* VS, 32-bit: use Microsoft's preferred I prefix */
# define SIZE_T_FMT "I32"
# define T_UINT64_FMT "I64"
# define T_INT64_FMT "I64"
# endif
# define LL_FMT "ll"
# define POINTER_FMT "p"
I moved the If that's good, I'll push a replacement. |
With the "z" prefix, whether
Yes, I know. I've been meaning to fix that. But with 19 PRs in the queue, it could be a while. (No, I don't have commit privs and that's probably a good idea because having a second set of eyes reviewing code is a good thing.)
I'd suggest using the "z" prefix for post-VS2015, "I32"/"I64" for VS-2015 and earlier. Don't worry about MinGW -- I'll fix that after this PR is merged. N.B.: MS is slowly converging on Clang, it would seem, with respect to C11 and C23 standard support. MS Visual Studio has been using N.N.B.: SIMH cannot be compiled with Clang on Windows due to interesting coding choices in the SLiRP support code. The SLiRP replacement, |
I ended up with some research time this afternoon, queried Perplexity.ai, which turns out to be quite useful and didn't turn out "botshit". Here's what I suggest, pending your concurrence:
Side note: According to Perplexity, VS 2010 will accept variable declarations anywhere, not just at the top of the function. So, there's a good chance the |
I can agree with just about all of that change, except that even on 32-bit platforms the Because So altogether the new conditional block would be: #if defined(_MSC_VER) || defined(__MINGW32__) || defined(__MINGW64__)
# if _MSC_VER >= 1900 || defined(__USE_MINGW_ANSI_STDIO)
/* VS 2015 or later, MinGW with ANSI stdio: Use the standard. Don't assume that
* MS or MinGW will support the "I" qualifier indefinitely. */
# define SIZE_T_FMT "z"
# define T_UINT64_FMT "j"
# define T_INT64_FMT "j"
# elif (_MSC_VER < 1900 && defined(_WIN64)) || defined(__MINGW64__)
/* VS, 64-bit: use Microsoft's "I" qualifier. */
# define SIZE_T_FMT "I64"
# define T_UINT64_FMT "I64"
# define T_INT64_FMT "I64"
# elif (_MSC_VER < 1900 && defined(_WIN32)) || defined(__MINGW32__)
# define SIZE_T_FMT "I32"
# define T_UINT64_FMT "I64"
# define T_INT64_FMT "I64"
#endif As with regards to the C standard VS 2010 (and older) support, Herb Sutter mentioned this on his blog (in 2012):
My own experience is that a lot of C99 code just works. But there's the occasional bit that blows up (oh hi there Unrelated to Visual Studio, but as you mentioned bringing in C99 libraries: That'll break the VAX build, because the HP C V6.4-005 compiler is not C99. From the vim v9.1 Reference Manual (which is from all of five days ago as of writing this -- July 22, 2025):
But that's an entirely different topic, unrelated to this PR. |
Updated with bscottm's suggested fixes to keep ANSI syntax for VS 2015 and later.
My bad. Even I get things wrong from time to time. :-) :-) :-)
Not much I can do about that -- I definitely can't ask the |
Per Microsoft documentation versions Visual Studio 2013 and below do not support the hh, j, z, and t length prefixes for printf format-type specifiers. Further, the preprocessor selection logic was modified so that 64-bit Visual Studio versions are actually recognized.