Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

first support for VS2015 #1341

Merged
merged 3 commits into from
Aug 25, 2015
Merged

first support for VS2015 #1341

merged 3 commits into from
Aug 25, 2015

Conversation

rainers
Copy link
Member

@rainers rainers commented Aug 8, 2015

This PR extracts internals of the VS runtime implementation of stdio into a separate module that can be implemented depending on the VS version.
This avoids having VS version specific conditionals in the public stdio module just leading to confusion when mixing prebuilt libraries with new code.

For now, this PR links the code for VS2013 or earlier versions, but the final version should put each module in a separate library. The compiler could then figure out what environment is used for linking and add the respective library to the link comand.

This PR works in combination with dlang/dmd#4879 by the compiler selecting the appropriate object file for the version of the LIBCMT library that will be used by link.exe.

@@ -75,6 +75,7 @@ SRCS=\
src\core\sys\windows\threadaux.d \
src\core\sys\windows\windows.d \
src\core\sys\windows\winsock2.d \
src\core\sys\windows\stdio_msvc12.d \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msvc14 missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATM you cannot link both stdio_msvc12 and stdio_msvc14 (this is the SRCS file). They both have to be added to MANIFEST, though.

@MartinNowak
Copy link
Member

For now, this PR links the code for VS2013 or earlier versions, but the final version should put each module in a separate library. The compiler could then figure out what environment is used for linking and add the respective library to the link comand.

Can't we add both version to druntime and depending on a version declaration of the compiler import the correct msvcxx modules. That way we don't need to ship multiple libraries.

@rainers
Copy link
Member Author

rainers commented Aug 9, 2015

Can't we add both version to druntime and depending on a version declaration of the compiler import the correct msvcxx modules. That way we don't need to ship multiple libraries.

My current idea is to add the new modules as libraries or object files to the lib64 folder while phobos64.lib is independent of the VS version. There is a compile switch or some automatic detection to add the appropriate library to the link. I'm not sure if the defaultlib-mechanism is appropriate here.

I suspect a solution using versions will cause a lot of confusion, because every prebuilt library needs to be compiled with the same VC version as the executable. Shipped as binaries will need two versions if all VC versions are supposed to be supported.

@MartinNowak
Copy link
Member

I suspect a solution using versions will cause a lot of confusion, because every prebuilt library needs to be compiled with the same VC version as the executable. Shipped as binaries will need two versions if all VC versions are supposed to be supported.

It also wouldn't work for druntim/phobos as DLL.

@MartinNowak
Copy link
Member

The defaultlib switch is fairly underpowered.
We could use an improved dmd.conf to select the appropriate libs.
#1341

@MartinNowak
Copy link
Member

I guess the easiest way forward is too hack support for VC detection into dmd's src/link.c.
We could then follow up with a nicer design later on.

@MartinNowak
Copy link
Member

From ldc-developers#29 (comment)

another reasonable use case for integer versions (version(CRuntime_Microsoft >= 14) ...)

@rainers
Copy link
Member Author

rainers commented Aug 11, 2015

Updated to build stdio_msvc*.d to object files. Now needs dlang/dmd#4879

@WalterBright
Copy link
Member

Rather than build a whole separate library, how about adding an extra .obj file that is linked in when using older VS?

@rainers
Copy link
Member Author

rainers commented Aug 11, 2015

Rather than build a whole separate library, how about adding an extra .obj file that is linked in when using older VS?

That's actually what this PR does now in combination with dlang/dmd#4879. It drags in a few symbols that are not always necessary, though (e.g. _flsbuf, _filbuf).

@MartinNowak
Copy link
Member

It drags in a few symbols that are not always necessary, though (e.g. _flsbuf, _filbuf).

Would it help to use libs instead of objects?

@rainers
Copy link
Member Author

rainers commented Aug 14, 2015

Would it help to use libs instead of objects?

I just verified that my expectation was unfounded. The functions in the object files are generated into COMDATs, so they are only linked if referenced.

@rainers
Copy link
Member Author

rainers commented Aug 14, 2015

It also wouldn't work for druntim/phobos as DLL.

Sharing the C runtime between phobos.dll and an application would pretty much only work if both link against the same version of the MSVCRnnn DLLs. So a phobos DLL will have to be rebuilt for every VS version.

@MartinNowak
Copy link
Member

Auto-merge toggled on

@MartinNowak
Copy link
Member

Update for the build script dlang/installer#139.

@MartinNowak
Copy link
Member

https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=1706290&isPull=true

d_do_test.obj : error LNK2019: unresolved external symbol _fputc_nolock referenced in function _D3std5stdio4File17LockingTextWriter10__T3putTwZ3putMFNfwZ12trustedFPUTCFNbNiNeiPS4core4stdc5stdio6_iobufZi
phobos64.lib(dmain2_61d_47b.obj) : error LNK2019: unresolved external symbol init_msvc referenced in function _d_run_main
phobos64.lib(stdio_b_408.obj) : error LNK2019: unresolved external symbol _fgetc_nolock referenced in function _D3std5stdio10readlnImplFPOS4core4stdc5stdio6_iobufKAawE3std5stdio4File11OrientationZm
d_do_test.exe : fatal error LNK1120: 3 unresolved externals

@rainers
Copy link
Member Author

rainers commented Aug 23, 2015

phobos64.lib(dmain2_61d_47b.obj) : error LNK2019: unresolved external symbol init_msvc referenced in function _d_run_main

The compiler has to select one of the new object files: dlang/dmd#4879

@yebblies
Copy link
Member

Auto-merge toggled off

@rainers
Copy link
Member Author

rainers commented Aug 24, 2015

I have added stdio_msvc12.d to the library build so that the PR should pass the tester. It should also let the dmd PR build successfully with VS2013 or earlier. Once the latter is merged we can remove the file from the lib to also build with VS2015.

MartinNowak added a commit that referenced this pull request Aug 25, 2015
@MartinNowak MartinNowak merged commit 478b6c5 into dlang:master Aug 25, 2015
@kinke
Copy link
Contributor

kinke commented Aug 26, 2015

I like the approach with the 2 object files. I've just compared it to my patch for LDC (kinke@6a1c9d3), and these are the differences.

Because of the lacking version declaration, constants cannot be adjusted for VS 2015+, at least as long as one wants to keep them as compile-time constants and not as __gshared globals:

  • TMP_MAX (short.max => int.max)
  • L_tmpnam (13 => 260)
  • _P_tmpdir isn't defined anymore
  • _wP_tmpdir neither
  • _IOREAD and friends neither

This PR keeps defining (v)snprintf as alias of _(v)snprintf. MSDN states:

vsnprintf is identical to _vsnprintf.

Scrap that. Looking at the inline definition in the 2015 stdio.h header, (v)snprintf uses the new C99-conformant mode (_CRT_INTERNAL_PRINTF_STANDARD_SNPRINTF_BEHAVIOR) and _(v)snprintf the legacy mode (_CRT_INTERNAL_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION).
This affects the output of floating point values and legacy _(v)snprintf doesn't terminate the string with null if is as long as or longer than the whole buffer and there's no space for the terminating null, instead of pruning the string and always emitting the null at the buffer end (standard-conformant). As you can imagine, that may make a huge difference. ;)
So in order to exploit the C99 conformant mode for VS2015+, we should declare (v)snprintf in stdio.d and implement it solely in stdio_msvc12.d by forwarding to legacy _(v)snprintf.

@rainers
Copy link
Member Author

rainers commented Aug 27, 2015

I think we should leave the constants as is ATM with comments that they apply to older version, but add alternatives with postfix _vc14 or similar for VS2015 and later.

So in order to exploit the C99 conformant mode for VS2015+, we should declare (v)snprintf in stdio.d and implement it solely in stdio_msvc12.d by forwarding to legacy _(v)snprintf.

I agree, we should use the C99 compliant version of snprintf as much as possible. Forwarding might also be done through /alternatename:snprintf=_snprint. See #1360

@kinke
Copy link
Contributor

kinke commented Aug 27, 2015

Cool, what a handy undocumented linker pragma. 👍

Forwarding might also be done through /alternatename:snprintf=_snprintf.

Exactly, a perfect fit!

I think we should leave the constants as is ATM with comments that they apply to older version, but add alternatives with postfix _vc14 or similar for VS2015 and later.

Hmm, then people may start using them and then we won't be able to get rid of them for quite a while. It would also encourage people to start/continue writing unportable, MS-specific code. I doubt there's much code relying on these constants out there in the D wilderness anyway, so I'd prefer keeping the constants (with comments) as-is for some time until we remove VS 2013- support from druntime.
Note that that step will allow us to get rid of quite a few special cases for MS runtimes, in both normal code as well as unit tests, and make druntime more streamlined once we can rely on a (mostly) C99-conformant host C runtime on all platforms...

Edit: #1361

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants