arximboldi / lager

C++ library for value-oriented design using the unidirectional data-flow architecture — Redux for C++
https://sinusoid.es/lager/
MIT License
704 stars 66 forks source link

Work around for MSVC compile error C3200 #177

Closed colugomusic closed 1 year ago

colugomusic commented 1 year ago

This works around the MSVC compiler error C3200. I have not tested this on Linux or macOS.

As far as I understand this is a MSVC bug (or there appears to be some disagreement between compilers on the correct way to interpret the standard.)

My understanding (which could be wrong) is that using reader_node in the base clause of store_node_base appears to change the interpretation of the identifier within the class itself (the name reader_node would instead be interpreted as reader_node<Model> because that is how it is instantiated inside root_node. Therefore base_t will be incorrectly defined as: root_node<Model, reader_node<Model> leading to MSVC incorrectly reporting that root_node is being instantiated with a non-template class for the second argument.

The only information I could find about this comes from this stackoverflow post: https://stackoverflow.com/questions/68000265/template-code-that-compiles-in-gcc-8-2-but-not-msvc-19

colugomusic commented 1 year ago

I am closing this because after recompiling my project without the workaround applied, the compile error is no longer occurring, I cannot for the life of me get the error to happen anymore, and I no longer trust my own sanity.

arximboldi commented 1 year ago

MSVC seems to work against the developer sanity. That's why I stay away from it. Thanks for the PR atttempt though @colugomusic ! Nice to see you here!

colugomusic commented 1 year ago

I have realized now my confusion here - This fix is required to compile in c++17 mode, but not in c++20. So I must have switched from 17 to 20 at some point while testing. C++20 also compiles fine with this fix. Also, the pull request I made earlier to immer (https://github.com/arximboldi/immer/pull/261) fixes c++20 mode but breaks c++17 (which compiles fine without the T:: fix.) There appears to be quite a few nasty differences between C++17 and C++20 which might require some #ifdefing to solve. I have been using an older version of immer in my c++17 project which has been working great but have run into a lot of issues when I tried to update to the latest version (maybe I should be upgrading my project to c++20 anyway though.)

arximboldi commented 1 year ago

Oh... :sweat: Damn MSVC. Is using Clang not an option for you? I hear it's not that difficult these days on Windows.

colugomusic commented 1 year ago

Oh... 😓 Damn MSVC. Is using Clang not an option for you? I hear it's not that difficult these days on Windows.

I suppose I could try but it would be nice to get things working with MSVC.