MSVC Build Error

[lehitoskin]

I pulled from master just a minute ago and tried running the 32-bit MSVC script, but was met with this error:

        cl -c /I..\include  /nologo /WX /EHsc /GL /Ox /Oy /Gy /DNDEBUG /MD /TC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS pngwutil.c
pngwutil.c
        del libpng.lib
Could Not Find C:\Users\lehi\Documents\c++\FLIF\build\MSVC\libpng-libpng-1.6.20-master-signed\libpng.lib
        lib /nologo /LTCG -out:libpng.lib png.obj pngerror.obj pngget.obj pngmem.obj pngpread.obj pngread.obj pngrio.obj pngrtran.obj pngrutil.obj pngset.obj p
ngtrans.obj pngwio.obj pngwrite.obj pngwtran.obj pngwutil.obj
        copy libpng.lib ..\x86_MD\\
        1 file(s) copied.
        copy png.h ..\include\\
        1 file(s) copied.
        copy pngconf.h ..\include\\
        1 file(s) copied.
        copy pnglibconf.h ..\include\\
        1 file(s) copied.
        cd ..
        cl /nologo /WX /EHsc /GL /Ox /Oy /Gy /DNDEBUG /MD /Fdx86_MD\Obj\\ /Fox86_MD\Obj\\ -c ..\..\src\flif-dec.cpp
flif-dec.cpp
..\..\src\flif-dec.cpp(218): error C2039: 'function': is not a member of 'std'
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\algorithm(14): note: see declaration of 'std'
..\..\src\flif-dec.cpp(218): error C2061: syntax error: identifier 'function'
..\..\src\flif-dec.cpp(222): error C2065: 'func': undeclared identifier
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.EXE"' : return code '0x2'
Stop.

Microsoft (R) Program Maintenance Utility Version 14.00.24210.0
Copyright (C) Microsoft Corporation.  All rights reserved.

        cl /nologo /WX /EHsc /GL /Ox /Oy /Gy /DNDEBUG /MD /Fdx86_MD\Obj\\ /Fox86_MD\Obj\\ -c ..\..\src\flif-dec.cpp
flif-dec.cpp
..\..\src\flif-dec.cpp(218): error C2039: 'function': is not a member of 'std'
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\algorithm(14): note: see declaration of 'std'
..\..\src\flif-dec.cpp(218): error C2061: syntax error: identifier 'function'
..\..\src\flif-dec.cpp(222): error C2065: 'func': undeclared identifier
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.EXE"' : return code '0x2'
Stop.
Microsoft Windows [Version 10.0.14393]
(c) 2016 Microsoft Corporation. All rights reserved.

[hrj]

std::function is part of the C++11 standard. Do you have to enable C++11 mode in MSVC?

[saschanaz]

MSVC 2015 supports C++11 without any parameter. (It supports C++14 by default)

[hrj]

@SaschaNaz so that requires MSVC++ 15 update 3.

What version is @lehitoskin using?

[saschanaz]

@hrj No, previous MSVC 2015 (VC15 means 2017) supported everything including C++17 by default. Starting from Update 3 it limits to C++14.

[hrj]

@SaschaNaz Oh ok. I haven't used MSVC in 20 years :smile: So I will leave it to the experts!

[saschanaz]

Probably one more \<functional> is needed in flif-dec.cpp? I cannot test right now as my machine currently only has VS2017.

[lehitoskin]

Ah, I'm using Visual Studio 14. I'll update to a newer version and try again.

[fherzog2]

SaschaNaz is right, there needs to be an #include <functional> in flif-dec.cpp.

Also, something I've noticed while looking at the compile error: This line seems strange: https://github.com/FLIF-hub/FLIF/blob/6d0ce089eab52623d6cc3ff3375b4ac57df65a69/src/flif-dec.cpp#L222

• A std::function object is casted into void* and stored for later use
• But at the end of {{issue_callback}} the std::function goes out of scope
• Later, when void* is casted back into std::function, it will access the stack at that location for undefined behaviour

[hrj]

Later, when void* is casted back into std::function, it will access the stack at that location for undefined behaviour

The access to the function is only supposed to happen during the issue_callback's lifecycle. In other words, callback_info_t is only valid within the call span of callback_t. Perhaps it needs better documentation, if it's not clear.

Apart from documentation, are there alternative ways to prevent dangling pointers in c++11?

[fherzog2]

I suppose the function is casted to void* because it appears in the API. The flif API usually uses opaque structs to avoid patterns like this. One can put all kinds of advanced C++ features into the struct that way. Maybe like the following?

// flif_dec.h

typedef struct callback_info_t callback_info_t;

FLIF_DLLIMPORT uint32_t FLIF_API flif_callback_info_get_quality(callback_info_struct* info);
FLIF_DLLIMPORT uint32_t FLIF_API flif_callback_info_get_bytes_read(callback_info_struct* info);
FLIF_DLLIMPORT void FLIF_API flif_decoder_generate_preview(callback_info_t *info);

// flif-interface_dec.cpp

struct callback_info_t {
    uint32_t quality;
    int64_t  bytes_read;

    // Private context
    std::function<void()> populateContext;
};

FLIF_DLLIMPORT uint32_t FLIF_API flif_callback_info_get_quality(callback_info_t* info) {
    return info->quality;
}

FLIF_DLLIMPORT uint32_t FLIF_API flif_callback_info_get_bytes_read(callback_info_t* info) {
    return info->bytes_read;
}

FLIF_DLLEXPORT void FLIF_API flif_decoder_generate_preview(callback_info_t *info) {
    try
    {
        info->populateContext();
    }
    catch(...) {}
}

[lehitoskin]

When I was updating to MSVC 15 I first uninstalled 14 because I thought, hey, 15 will do what I want anyway. I have made a grave mistake and everything is ruined forever. The new installer is rather confusing and doesn't exactly tell you what is going to be installed, even if you look at the itemized list. It seems that the dl_make_vs.bat script still calls for MSVC 14, so I reinstalled that, but now I'm getting errors like these and I have no idea why:

PS C:\Users\lehi\Documents\c++\FLIF\build\MSVC> .\dl_make_vs.bat
Error in script usage. The correct usage is:
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" [option]
  or
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" [option] store
  or
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" [option] [version number]
  or
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" [option] store [version n
umber]
where [option] is: x86 | amd64 | arm | x86_amd64 | x86_arm | amd64_x86 | amd64_arm
where [version number] is either the full Windows 10 SDK version number or "8.1" to use the windows 8.1 SDK
:
The store parameter sets environment variables to support
  store (rather than desktop) development.
:
For example:
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x86_amd64
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x86_arm store
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x86_amd64 10.0.10240.0
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x86_arm store 10.0.10240.
0
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x64 8.1
    "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\..\..\VC\vcvarsall.bat" x64 store 8.1
:
Please make sure either Visual Studio or C++ Build SKU is installed.
need VS2015
Microsoft Windows [Version 10.0.14393]
(c) 2016 Microsoft Corporation. All rights reserved.

[hrj]

The flif API usually uses opaque structs to avoid patterns like this. One can put all kinds of advanced C++ features into the struct that way.

Is C compatibility really required? If a C++ API for FLIF would be sufficient, we can save developer overhead in maintaining this bridge.

[bjorn3]

That would prevent usage with many languages (python, rust, go, ...).

[saschanaz]

I'm currently using C API on my library. Binding on C++ API is more complex with embind :/

[saschanaz]

@lehitoskin That's because the current script only supports VS2015. PR to support VS2017+: #397

[lehitoskin]

@hrj I also rely on the C FFI for Racket in Ivy.

[hrj]

I suppose the function is casted to void* because it appears in the API.

Yes and also because std::function can't be represented in C.

The flif API usually uses opaque structs to avoid patterns like this. One can put all kinds of advanced C++ features into the struct that way. Maybe like the following?

Yup. I broke the "opaque structs" pattern. Either we could add those extra functions to get the struct's members like @lehitoskin mentioned, or I can rollback the need for a structure itself. Since there are only three members (quality, bytes_read and the void * pointer), we can just pass them as parameters to the callback and be done with it. (I had initially thought that there would be more than three parameters so I created a structure).