Skip to content
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

MSVC: C2440 compiler error merging deps #126

Open
kevin-- opened this issue Dec 9, 2021 · 23 comments
Open

MSVC: C2440 compiler error merging deps #126

kevin-- opened this issue Dec 9, 2021 · 23 comments

Comments

@kevin--
Copy link

kevin-- commented Dec 9, 2021

In deps.hpp, we see this method starting at line 399

    template <typename... Ds>
    auto merge(deps<Ds...> other)
    {
        return boost::hana::unpack(
            boost::hana::union_(spec_map, deps<Ds...>::spec_map),
            [&](auto... ts) {
                using deps_t =
                    deps<std::decay_t<decltype(boost::hana::second(ts))>...>;
                return deps_t{*this, std::move(other)};
            });
    }

building with MSVC 2019 16.11.3, return deps_t{*this, std::move(other)}; causes the error message

C2440	'<function-style-cast>': cannot convert from 'initializer list' to 'deps_t'

with this hint in the log

2> message : No constructor could take the source type, or constructor overload resolution was ambiguous

same code compiles and runs good on AppleClang.

I'm not very familiar with boost::hana so some tips to troubleshoot would be appreciated. It seems that perhaps deps_t is not ending up as the expected type?

This error is encountered with any number of deps greater than 0

@arximboldi
Copy link
Owner

I'm not very familiar with MSVC, but I heard recently from a client that Lager started building with MSVC when they enabled C++20 on a recent version.

@kevin--
Copy link
Author

kevin-- commented Dec 9, 2021

Tried building with C++20 (was building with C++17 and /permissive- before), and the error remains.

Intellisense shows the available overloads for deps_t as default ctor, copy ctor, or move ctor; nothing with 2 arguments. Might be a red herring but seems like the resolved type is not a pair? (if I'm understanding the code correctly)

@arximboldi
Copy link
Owner

I think you need MSVC 2021 for it to work. I still have a meeting pending with this client that made it work, I will report back when I get to know more about that.

@carsten-grimm-at-ipolog
Copy link
Contributor

Hi @kevin--, I believe my pull request #127 might help with this issue.

With this pull request, I could build and run all examples were I have all dependencies (i.e., nothing using sdl, imgui, libhttpserver or curses).

Also, I can build and run all test. However, one of the tests test-event_loop-queue is failing with MSVC. It appears that some lambda capture failed and a method is called on an uninitialized object: At some point in the call stack, I have a this pointer with the memory address 0xddddd... that MSVC uses to indicate uninitialized memory in debug mode. I could not pinpoint the cause of this issue and would appreciate any help. Thank you.

As @arximboldi mentioned, I am using the version of MSVC that ships with Visual Studio 2022.

@kevin--
Copy link
Author

kevin-- commented Jan 12, 2022

@carsten-grimm-at-ipolog thank you for doing great work! We have been building with C++20 and I wrote a workaround class to manage dependencies as a global 😿 . Hopefully I will get a chance to try your changes 🙏

@menzels
Copy link

menzels commented May 24, 2022

I am also using MSVC 2022 and wanted to try lager, but it seems it does not build anymore.

Is anyone still using MSVC and also interested in fixing this?

@arximboldi
Copy link
Owner

What kind of issue are you expderiencing @menzels ?

@menzels
Copy link

menzels commented May 24, 2022

compiling the tests i get the following errors:

static_assert failed: 'std::function only accepts function types as template arguments.'

'type': is not a member of 'std::_Get_function_impl<_Fty>'

'type': base class undefined

syntax error: identifier 'type'

'_Mybase': is not a class or namespace name

'unknown-type': is not a valid type for non-type template parameter '__formal'

'return': cannot convert from 'std::pair<lager::debugger<action_t,model_t,deps_t>::model,lager::effect<std::variant<Action,lager::debugger<Action,model_t,deps_t>::goto_action,lager::debugger<Action,model_t,deps_t>::undo_action,lager::debugger<Action,model_t,deps_t>::redo_action,lager::debugger<Action,model_t,deps_t>::pause_action,lager::debugger<Action,model_t,deps_t>::resume_action>,lager::deps<lager::dep::ref<T>>>>' to 'lager::debugger<action_t,model_t,deps_t>::model'

'auto lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_1>::operator ()<lager::effect<std::variant<Action,lager::debugger<Action,model_t,deps_t>::goto_action,lager::debugger<Action,model_t,deps_t>::undo_action,lager::debugger<Action,model_t,deps_t>::redo_action,lager::debugger<Action,model_t,deps_t>::pause_action,lager::debugger<Action,model_t,deps_t>::resume_action>,lager::deps<>>&>(_T1) const': function cannot access 'lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::ctx'

build log with these errors:
https://pastebin.com/Pe20W9T0

@arximboldi
Copy link
Owner

I'll wait for one of the Windows developers to chim in :)

@frgp42
Copy link

frgp42 commented Jul 3, 2022

@menzels I believe the build issues are something to do with how msvc handles std::enable_if_t but I'm not sure of the exact issue. I'm not a professional developer and no templates expert. To work around this I have been using the following where ever neccessary:

#ifdef _MSC_VER
    template <typename Fn>
    requires std::is_same_v<void, std::invoke_result_t<Fn&, const context_t&>>
#else
    template <
        typename Fn,
        std::enable_if_t<std::is_same_v<void, std::invoke_result_t<Fn&, const context_t&>>,
                         int> = 0>
#endif

I'm using VS2022 v17.2.5 and I can successfully run the tests.

This needs c++20 so would only work for msvc and breaks the minimum c++17 requirement for everything else.

I'm hesitant to put this forward as a PR as it seems to be a msvc bug and I don't think it's a good idea to change code permanently for a possibly temporary issue.

@arximboldi
Copy link
Owner

Thank you a lot @frgp42 ! I do not have a Windows machine at hand so I can't really explore this much further, but I'd welcome a PR from someone that knows what's going on there. I agree thought that having C++20 as requirement feels like too much, and I'd rather keep the library C++17 compatible on all platforms.

@arximboldi
Copy link
Owner

@carsten-grimm-at-ipolog did some work on porting to Windows, do you know what's going on?

@timpatt
Copy link
Contributor

timpatt commented Oct 24, 2022

I've seen some of the same issues on Visual Studio 2019 v16.10.3:

  1. auto merge(deps<Ds...> other) - this can be fixed by adding #define LAGER_DISABLE_STORE_DEPENDENCY_CHECKS. VS seems to hide the required constuctor because the associated std::enable_if doesn't get evaluated correctly. This define should probably be set whenever we're using the msvc compiler. I suspect it is currently set for CMake-based lager installs (in the CMake script), but it might be worthwhile considering conditionally setting it by default (for VS compiler) in the config.hpp so that just copying the lager headers into your project works correctly.
  2. "ctx" not being found seems to be a bug with VS evaluating the declspec(eff(ctx)) in the nested lambdas in store_node::dispatch. The fix is to simply add auto& ctxVisualStudioWorkaround = ctx and use declspec(eff(ctxVisualStudioWorkaround)) instead.

@arximboldi
Copy link
Owner

@timpatt I'd be happy to accept a PR with any of those fixes.

@arximboldi
Copy link
Owner

Thanks a lot for debugging these MSVC issues.

@timpatt
Copy link
Contributor

timpatt commented Oct 27, 2022

My pleasure! Thanks for such an amazing set of libraries! We're about to head into a long weekend here but will look at raising a PR mid-next week...

@kevin--
Copy link
Author

kevin-- commented Nov 1, 2022

just updating from 5feb9c7 to HEAD (11b4326) and getting the same error as shown here #126 (comment)

Previously building successfully with VS2019 and VS2022 with C++20 enabled and not using deps.

Did enable LAGER_DISABLE_STORE_DEPENDENCY_CHECKS but same result.

Is this the fix you are suggesting @timpatt (in store_node::dispatch )? Did not work for me...

 +                             auto& vsWorkaroundCtx = ctx;
                               if constexpr (std::is_same_v<void,
 -                                                          decltype(eff(ctx))>) {
 +                                                          decltype(eff(vsWorkaroundCtx))>) {

@kevin--
Copy link
Author

kevin-- commented Nov 1, 2022

Bisecting indicates that the origin/futures work seemed to break MSVC in this fashion -- see c87a4c7 is the first mainline commit that doesn't build for me

@arximboldi
Copy link
Owner

I don't really have a way to try MSVC, but I'd be happy to accept a fix from any of you @kevin-- @timpatt

@timpatt timpatt mentioned this issue Nov 28, 2022
@timpatt
Copy link
Contributor

timpatt commented Nov 28, 2022

Hi all. Sorry for the (very) late reply.

I've raised the (very small) PR that captures the changes that I made to fix the build - at least with my minimal test case.

@kevin--, do you have a minimal repro that I could try and look at? I haven't been able to reproduce it myself... I'm no expert, but I could have a play around at least!

@bobkocisko
Copy link

bobkocisko commented Apr 28, 2023

In case anyone else is interested in a workaround which is resolving all the build issues for me in Visual Studio: install "C++ Clang tools for Windows" in the vs installer and then switch the Platform Toolset in the project properties to "LLVM (clang-cl)". So far I haven't encountered any downsides to this. As far as I understand, it's still building a native windows app, linked against the windows runtime libraries, but compiling with clang.

@bobkocisko
Copy link

bobkocisko commented Apr 28, 2023

Or, if you are staying in the msvc compiler realm, be sure that the /permissive- compiler option is turned on which enables proper standards conformance. Apparently it is normally turned on, but I am using the JUCE framework's Projucer to launch Visual Studio and it does not turn that setting on. Until I had found and enabled that setting, I had all kinds of additional problems besides the ones described in this thread. But with it turned on I was able to follow the suggestions from this thread to successfully compile in msvc.

All that said, I'm preferring clang because it just happily builds lager as-is with no source modifications. It also builds significantly faster than msvc in my experience.

@gracicot
Copy link
Contributor

I'm now getting fatal error C1057: '{': no matching token found when applying the fixes. It seems the latest version of MSVC has a hard time counting braces when std::function is involved.

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

No branches or pull requests

8 participants