kicad-developers team mailing list archive

[Simon Richter <Simon.Richter@xxxxxxxxxx>]

Hi,

On 12.03.2018 19:10, Wayne Stambaugh wrote:

> As long as we don't have to add msvs specific fixes to the kicad source
> then I don't have any issues with this but I think this falls under the
> category be careful what you wish for.

There is one major issue for building with Visual Studio: static
libraries are not linked elementwise, but rather, their entire symbol
table is imported in one go, and sections pulled in as required.

This means that symbols need to be unique for the entire project, and
overwriting symbols from static libraries (as the qa programs do) does
not work.

I've avoided the issue by simply removing the "qa" subfolder from the
main CMakeLists.txt, but obviously that is not a solution.

> The context switching build
> issue will most likely have to be fixed yet again if we choose to go
> down this path.  I do not believe current context assembly files will
> build with msvc so we will either have to provide both sets of assembly
> files or go back to using them from the boost which has it's own set of
> well known build issues.

FWIW, I've backed out the relevant commits, and it builds fine with
Boost 1.60 and 1.62. The only problematic version seems to be 1.61. If
we wanted to go back to Boost, we could perhaps special-case 1.61 if
there is significant demand.

The only other MSVC specific change is a workaround for a VS2015
internal compiler error. The code in question is nonsensical anyway,
I've replaced it with equally nonsensical code that is a bit more
legible so it is more obvious that it is nonsensical. For those who
care: it's the initializers for the BOX2<T> template, which use
std::numeric_limits<T> to default-initialize boxes to span the largest
possible area. Except this doesn't work, because

    using namespace std;
    cout << numeric_limits<int>::min() << ' '
        << numeric_limits<int>::max() << endl;
    cout << numeric_limits<float>::min() << ' '
        << numeric_limits<float>::max() << endl;

gives

    -2147483648 2147483647
    1.17549e-38 3.40282e+38

so the default initialization for BOX2<int> covers (0,0) while
BOX2<float> does not. My suspicion is that this is either responsible
for some very subtle bugs, or simply does not matter because all
default-initialized boxes are later overwritten anyway.

Current MSVC patch stack:

    Remove qa code for MSVC
    Remove own copy of FindOpenSSL.cmake
    Revert "Switched coroutine library from Boost libcotext to custom
libcontext."
    Revert "Made libcontext compile on FreeBSD. Code formatting too."
    Revert "Preserve FPU registers on context switch"
    Revert "libcontext FreeBSD compatibility fix"
    Revert "Some coverity fixes"
    Revert "Fix an initialization order warning in coroutine.h"
    Work around VC++ 2015 bug.
    Define compiler flags for MSVC

IMO:

 - the compiler flags patch can be applied as is, it just adds a
conditional block to CMakeLists.txt
 - the workaround can be applied as is, or someone can look at that code
and throw it out
 - the context stuff needs to be decided
 - the OpenSSL CMake script can probably be removed and the shipped
version used, but we need to check why we had this file in the first place
 - the QA code needs refactoring if we want to compile it with MSVC, so
the best we can do now is disable it on MSVC.

   Simon

Attachment: signature.asc
Description: OpenPGP digital signature