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: Fixes #166

Merged
merged 2 commits into from
Mar 26, 2024
Merged

MSVC: Fixes #166

merged 2 commits into from
Mar 26, 2024

Conversation

timpatt
Copy link
Contributor

@timpatt timpatt commented Nov 28, 2022

Addresses (some?) of the issues raised in #126 .

The CMake export script correctly sets the `LAGER_DISABLE_STORE_DEPENDENCY_CHECKS` define when using MSVC, however this value is not set when using lager as a raw include (ie. without CMake).
There seems to be a bug with MSVC when evaluating `declspec(eff(ctx))` in the nested lambdas in store_node::dispatch.
Comment on lines +172 to -173
auto& ctxMsvcWorkaround = ctx;
if constexpr (std::is_same_v<void,
decltype(eff(ctx))>) {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of binding ctx to ctxMsvcWorkaround, have you tried using this->ctx in the decltype expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - I tried that first... It fails with the original error:

Error Report
D:\Work\cpp_ui\lager\lager/store.hpp(174,56): error C2039: 'ctx': is not a member of 'lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_1>' [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(196): message : see declaration of 'lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_1>' [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\lager\lager/effect.hpp(224): message : see reference to function template instantiation 'auto lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_1>::operator ()<_This&>(_T1) const' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>,
              _This=lager::effect<Action,lager::deps<Effect &>>,
              _T1=lager::effect<Action,lager::deps<Effect &>> &
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(196): message : see reference to function template instantiation 'immer::vector<const std::string,immer::default_memory_policy,5,2> lager::invoke_reducer<lager::deps<lager::dep::ref<T>,lager::dep::val<lager::with_debugger::<lambda_1>>>,lager::default_reducer_t&,const immer::vector<const std::string,immer::default_memory_policy,5,2>&,Action,lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_1>,lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch::<lambda_2>::()::<lambda_2>>(Reducer,Model,Action &&,EffectHandler &&,NoEffectHandler &&)' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              T=Effect,
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>,
              Reducer=lager::default_reducer_t &,
              Model=const immer::vector<const std::string,immer::default_memory_policy,5,2> &,
              Action=Action,
              EffectHandler=lager::store<action_t,model_t,deps_t>::store_node<lager::default_reducer_t,lager::with_manual_event_loop,deps_t,boost::hana::set<boost::hana::type_impl<void>::_>>::dispatch::<lambda_2>::()::<lambda_1>,
              NoEffectHandler=lager::store<action_t,model_t,deps_t>::store_node<lager::default_reducer_t,lager::with_manual_event_loop,deps_t,boost::hana::set<boost::hana::type_impl<void>::_>>::dispatch::<lambda_2>::()::<lambda_2>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(150): message : while compiling class template member function 'lager::future lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>::dispatch(Action)' [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\xmemory(1893): message : see reference to class template instantiation 'lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\memory(2027): message : see reference to class template instantiation 'std::_Wrap<_Ty>' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              _Ty=lager::store<action_t,model_t,deps_t>::store_node<lager::default_reducer_t,lager::with_manual_event_loop,deps_t,boost::hana::set<boost::hana::type_impl<void>::_>>
          ]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\memory(2725): message : see reference to class template instantiation 'std::_Ref_count_obj2<_Ty>' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              _Ty=lager::store<action_t,model_t,deps_t>::store_node<lager::default_reducer_t,lager::with_manual_event_loop,deps_t,boost::hana::set<boost::hana::type_impl<void>::_>>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(87): message : see reference to function template instantiation 'std::shared_ptr<lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>> std::make_shared<lager::store<action_t,model_t,deps_t>::store_node<ReducerFn,EventLoop,Deps,Tags>,immer::vector<const std::string,immer::default_memory_policy,5,2>,lager::default_reducer_t,lager::with_manual_event_loop,lager::deps<lager::dep::ref<T>,lager::dep::val<lager::with_debugger::<lambda_1>>>>(immer::vector<const std::string,immer::default_memory_policy,5,2> &&,lager::default_reducer_t &&,lager::with_manual_event_loop &&,lager::deps<lager::dep::ref<T>,lager::dep::val<lager::with_debugger::<lambda_1>>> &&)' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>,
              T=Effect
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(376): message : see reference to function template instantiation 'lager::store<action_t,model_t,deps_t>::store<lager::default_reducer_t,_T19,_T14,_T15>(immer::vector<const std::string,immer::default_memory_policy,5,2>,ReducerFn,EventLoop,Deps,Tags)' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              _T19=lager::with_manual_event_loop,
              _T14=deps_t,
              _T15=boost::hana::set<boost::hana::type_impl<void>::_>,
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(371): message : see reference to function template instantiation 'lager::store<action_t,model_t,deps_t>::store<lager::default_reducer_t,_T19,_T14,_T15>(immer::vector<const std::string,immer::default_memory_policy,5,2>,ReducerFn,EventLoop,Deps,Tags)' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              _T19=lager::with_manual_event_loop,
              _T14=deps_t,
              _T15=boost::hana::set<boost::hana::type_impl<void>::_>,
              ReducerFn=lager::default_reducer_t,
              EventLoop=lager::with_manual_event_loop,
              Deps=deps_t,
              Tags=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(260): message : see reference to function template instantiation 'lager::store<action_t,model_t,deps_t> lager::make_store::<lambda_1>::operator ()<_T16,_T17,const lager::default_reducer_t&,_T19,deps_t,_T21>(_T10,_T11 &&,_T12,_T13 &&,_T14 &&,_T15 &&) const' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              _T16=lager::type_<Action>,
              _T17=State,
              _T19=lager::with_manual_event_loop,
              _T21=boost::hana::set<boost::hana::type_impl<void>::_>,
              _T10=lager::type_<Action>,
              _T11=State,
              _T12=const lager::default_reducer_t &,
              _T13=lager::with_manual_event_loop,
              _T14=deps_t,
              _T15=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\lager\lager/store.hpp(360): message : see reference to function template instantiation 'lager::store<action_t,model_t,deps_t> lager::with_deps::<lambda_1>::()::<lambda_1>::operator ()<lager::type_<Action>,_Ty,const lager::default_reducer_t&,lager::with_manual_event_loop,lager::deps<>,boost::hana::set<boost::hana::type_impl<Tag>::_>>(_T16,_T17 &&,_T18,_T19 &&,_T20 &&,_T21 &&) const' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              Action=Action,
              _Ty=State,
              Tag=void,
              _T16=lager::type_<Action>,
              _T17=State,
              _T18=const lager::default_reducer_t &,
              _T19=lager::with_manual_event_loop,
              _T20=lager::deps<>,
              _T21=boost::hana::set<boost::hana::type_impl<void>::_>
          ]
D:\Work\cpp_ui\main.cpp(45): message : see reference to function template instantiation 'lager::store<action_t,model_t,deps_t> lager::make_store<Action,void,State,lager::with_manual_event_loop,lager::with_deps::<lambda_1>>(Model &&,EventLoop &&,lager::with_deps::<lambda_1> &&)' being compiled [D:\Work\cpp_ui\build\cpp_ui.vcxproj]
          with
          [
              Model=State,
              EventLoop=lager::with_manual_event_loop
          ]

That's why I suspect this is a compiler bug... It seems to fail when referring to "this" from within an unevaluated context within a multi-level lambda. I suspect that's why it does compile when you pull the reference into the local scope with the workaround.

I'm still not a huge fan of this solution, but it works 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...it's getting late tonight, but I'll try and build the failing tests @kevin-- mentioned in the next day or two.

Copy link

Choose a reason for hiding this comment

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

for what its worth, this error does not seem to appear with your work around. So step by step 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't std::invoke_result work instead of the decltype on the entire expression?
Can't really test myself anymore. We have had a lot of compiler bugs with older MSVCs and ended up switching to clang-cl that just works...

@arximboldi
Copy link
Owner

@kevin--, can you try this PR see if this fixes your isses?

@codecov-commenter
Copy link

Codecov Report

Merging #166 (e0090b3) into master (24887ac) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #166   +/-   ##
=======================================
  Coverage   94.30%   94.30%           
=======================================
  Files          80       80           
  Lines        2616     2617    +1     
=======================================
+ Hits         2467     2468    +1     
  Misses        149      149           
Impacted Files Coverage Δ
lager/store.hpp 96.87% <100.00%> (+0.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kevin--
Copy link

kevin-- commented Nov 29, 2022

I'm receiving a compilation error regarding effect, seems that the parent class type of effect is not being computed correctly?

2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1010,19): error C2338: static_assert failed: 'std::function only accepts function types as template arguments.'
2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1034): message : see reference to class template instantiation 'std::_Get_function_impl<_Fty>' being compiled
2>        with
2>        [
2>            _Fty=counter::action
2>        ]
2>\lib\lager\lager/effect.hpp(81): message : see reference to class template instantiation 'std::function<counter::action>' being compiled
2>\lib\lager\test\core.cpp(205): message : see reference to class template instantiation 'lager::effect<counter::action,lager::deps<services::foo &>>' being compiled
2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1034,51): error C2039: 'type': is not a member of 'std::_Get_function_impl<_Fty>'
2>        with
2>        [
2>            _Fty=counter::action
2>        ]

when building test-core, as well as my own project.

Somehow it seems to be deducing the type of the parent class as std::function<counter::action> which of course then fails the static assert since it is not a callable signature.

@bobkocisko
Copy link

I'm receiving a compilation error regarding effect, seems that the parent class type of effect is not being computed correctly?

2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1010,19): error C2338: static_assert failed: 'std::function only accepts function types as template arguments.'
2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1034): message : see reference to class template instantiation 'std::_Get_function_impl<_Fty>' being compiled
2>        with
2>        [
2>            _Fty=counter::action
2>        ]
2>\lib\lager\lager/effect.hpp(81): message : see reference to class template instantiation 'std::function<counter::action>' being compiled
2>\lib\lager\test\core.cpp(205): message : see reference to class template instantiation 'lager::effect<counter::action,lager::deps<services::foo &>>' being compiled
2>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\functional(1034,51): error C2039: 'type': is not a member of 'std::_Get_function_impl<_Fty>'
2>        with
2>        [
2>            _Fty=counter::action
2>        ]

when building test-core, as well as my own project.

Somehow it seems to be deducing the type of the parent class as std::function<counter::action> which of course then fails the static assert since it is not a callable signature.

I'm having the same problem. Has anyone found a solution for this? It's super confusing that msvc thinks that the model struct itself is the function signature.

@heejune
Copy link

heejune commented May 15, 2023

@bobkocisko I also struggled with MSVC treating the given action as std::function arguments, but I managed to compile it using the following workaround.

from lager/effect.hpp

template <typename Fn>
struct func : public std::function<Fn>
{};

template <typename Action, typename Deps = lager::deps<>>
struct effect : public func<future(const lager::context<Action, Deps>&)>

However, MSVC still shows one remaining build failure issue where the static_assert fails.

    static_assert(decltype(boost::hana::length(result))::value > 0, "");

@timpatt
Copy link
Contributor Author

timpatt commented Dec 10, 2023

I've raised an issue with Microsoft regarding the MSVC compile error with "effect". It worked in VC 19.31 and prior.

@arximboldi
Copy link
Owner

Is this PR still useful to any of the MSVC users? Should we merge it or ditch it? @kevin-- @jarzec @timpatt ?

@jarzec
Copy link
Contributor

jarzec commented Feb 7, 2024

Is this PR still useful to any of the MSVC users? Should we merge it or ditch it? @kevin-- @jarzec @timpatt ?

As I mentioned at some stage we have moved from MSVC to clang so I cannot give much input on MSVC anymore.

@kevin--
Copy link

kevin-- commented Feb 26, 2024

I haven't successfully tested this branch, but does seem like a step forward.

@arximboldi arximboldi merged commit 503ea6a into arximboldi:master Mar 26, 2024
@arximboldi
Copy link
Owner

Merged. Thanks a lot @timpatt for helping making things work on MSVC!

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

Successfully merging this pull request may close these issues.

7 participants