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

core.stdc.fenv: proper support for Microsoft C Runtime (Visual Studio >= 2013)#969

Merged
rainers merged 3 commits intodlang:masterfrom
kinke:fenv
Mar 27, 2015
Merged

core.stdc.fenv: proper support for Microsoft C Runtime (Visual Studio >= 2013)#969
rainers merged 3 commits intodlang:masterfrom
kinke:fenv

Conversation

@kinke
Copy link
Contributor

@kinke kinke commented Sep 27, 2014

VCRT 2013 uses a minimally different feraiseexcept() implementation than this one from VCRT 2014 CTP 3 (2013 additionally sets the exception flags via fesetexceptflag), but I guess that doesn't make a huge difference. The rest of 2013 is identical to 2014, so MSVC 2013 should be supported too.
More important are the correct constants and the implementation of 2 inline functions (to prevent linking errors).

@kinke
Copy link
Contributor Author

kinke commented Sep 27, 2014

The failing Win64 autotest just illustrates that currently declared functions (in this case, fesetenv and fetestexcept declared in the same module) are not available in the linked C runtime (presumably MSVC < 2013) and thus lead to linking errors when used.
It compiles with MSVC >= 2013 though.

@kinke kinke changed the title core.stdc.fenv: proper support for Microsoft C Runtime (Visual Studio >= 2014) core.stdc.fenv: proper support for Microsoft C Runtime (Visual Studio >= 2013) Oct 11, 2014
@andralex
Copy link
Member

Please rebase. Windows experts n da house who can review this?

Copy link
Member

Choose a reason for hiding this comment

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

You should also use CRuntime_DigitalMars for the version condition just above this one. DMC_RUNTIME is no longer correct.

@kinke
Copy link
Contributor Author

kinke commented Mar 21, 2015

PING! It's almost been half a year now and I don't see any reason not to merge this.

@redstar
Copy link
Contributor

redstar commented Mar 22, 2015

I like to see this merged, too. It supports the Win64 port of LDC.

@rainers
Copy link
Member

rainers commented Mar 27, 2015

I guess the resistence to merging comes from the impression that the change only works for VS2013 and later. IIUC this is not the case, though some C99 functions are not available in earlier versions and will cause link errors.
The change of the return values affect other platforms, too, but they seem correct according to cplusplus.com.

@rainers
Copy link
Member

rainers commented Mar 27, 2015

VCRT 2013 uses a minimally different feraiseexcept() implementation than this one from VCRT 2014 CTP 3 (2013 additionally sets the exception flags via fesetexceptflag), but I guess that doesn't make a huge difference.

The VS2013 seems to unmask the requested exceptions, so it is an actual change, but let's consider the VS2015 version a bug fix. The older version didn't even compile for C code: https://connect.microsoft.com/VisualStudio/feedback/details/806624/fenv-h-unusable-in-vs-2013

@rainers
Copy link
Member

rainers commented Mar 27, 2015

It'd be nice if there would be some tests for this, though I understand that's a bit difficult if they depend on the version of the MS runtime.

@rainers
Copy link
Member

rainers commented Mar 27, 2015

Auto-merge toggled on

rainers added a commit that referenced this pull request Mar 27, 2015
core.stdc.fenv: proper support for Microsoft C Runtime (Visual Studio >= 2013)
@rainers rainers merged commit 38c8805 into dlang:master Mar 27, 2015
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.

4 participants