Windows MSVC errors

[elushaX]

i build library on windows and had some test errors including my own application after comparing linux build and windows build side by side i found the reason for this issue

if this is something that should be fixed and pushed to the repository i will create a pull request

linux (clang and gcc)

windows (msvc)

the issue

[elushaX]

Relative commit from my fork (including some additional personal tweaks that shall or shall not be added to the pull request) https://github.com/cwbaker/lalr/compare/main...elushaX:lalr:main

[cwbaker]

Hi Ilya,

That does look like something I'd be happy to take. Thanks very much for taking the time to try the library out and fix a problem.

Could you please format the initializer list so that it matches the existing style, e.g. commas at the start of the line with no indentation? I'll happily accept a PR with that fix.

I'd also be happy for a separate PR to add CMake build scripts if you would be happy for me to forward any queries about that to you?

Thanks, Charles

On Tue, 11 Jun 2024 at 21:39, Ilya Shurupov @.***> wrote:

i build library on windows and had some test errors including my own application after comparing linux build and windows build side by side i found the reason for this issue

if this is something that should be fixed and pushed to the repository i will create a pull request

linux (clang and gcc) image.png (view on web) https://github.com/cwbaker/lalr/assets/163508118/bc4efc04-c2fc-4e42-a0a8-60a40127f08e

windows (msvc) image.png (view on web) https://github.com/cwbaker/lalr/assets/163508118/c3ab5cbf-f6d7-4462-94e3-4adb26788e55

the issue image.png (view on web) https://github.com/cwbaker/lalr/assets/163508118/51c15457-294d-4c75-8fd6-0f303427d9ea

— Reply to this email directly, view it on GitHub https://github.com/cwbaker/lalr/issues/54, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFGJWZGZTH2VKX4GVDPYODZG3AVTAVCNFSM6AAAAABJD6ORYKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DKOJSGMYTMOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[elushaX]

Hi Charles,

I've made PR regarding the bug. I'd be happy to know why this issue is only apparent on the MSVC compiler but not on others. Do you have any thoughts or resources for me to consider?

I'd also be happy for a separate PR to add CMake build scripts if you would be happy for me to forward any queries about that to you?

Sure, I also have made PR for that.

[elushaX]

Also, I want to say big thanks for such a library. I use it in my project and love it. If you are interested you can check out the usage here

The only thing I didn't figure out is how to properly destruct failed roots on sentence rejection so I made the ugly workaround src/lalr/Parser.ipp

This is probably just my lack of understanding of the library (use shared pointers inside user node?), but if you think this is something that is potentially missed from the library perspective - I will spend more time on that and report it later again.

[cwbaker]

I've made PR regarding the bug. I'd be happy to know why this issue is only apparent on the MSVC compiler but not on others. Do you have any thoughts or resources for me to consider?

Thanks! I've merged that now. I'm unsure exactly why that problems occurs on MSVC but not other compilers -- it has no error on some MSVC versions, including the version provided by Github Actions which is why I missed it previously.

If I had to guess I would say that the compiler provided STL avoids the incorrect constructors but I haven't dug in and confirmed that. That'd be the first thing I'd look at.

I'd also be happy for a separate PR to add CMake build scripts if you would be happy for me to forward any queries about that to you?

Sure, I also have made PR for that.

Thanks again, merged that too.

[cwbaker]

Also, I want to say big thanks for such a library. I use it in my project and love it. If you are interested you can check out the usage here

No worries. Thanks for taking the time to add some improvements and, especially, for getting in touch and letting me know. It's great seeing the library get some use. Oscript looks neat, building a scripting language is a great project.

The only thing I didn't figure out is how to properly destruct failed roots on sentence rejection so I made the ugly workaround src/lalr/Parser.ipp

This is probably just my lack of understanding of the library (use shared pointers inside user node?), but if you think this is something that is potentially missed from the library perspective - I will spend more time on that and report it later again.

The idea is that you use shared_ptr<> or types with value semantics for UserData. From looking at your UserNode in Modules/Objects/.../parser/Private.hpp using a shared_ptr seems reasonable. Another example is ShaderParser.cpp from my reyes project where I do something similar; I have expressions and statements represented by a single SyntaxNode class but otherwise pretty similar to your approach.

extern const lalr::ParserStateMachine* shader_parser_state_machine;
lalr::Parser<lalr::PositionIterator<Iterator>, shared_ptr<SyntaxNode>, char> parser( shader_parser_state_machine, this );

Thanks and HTH, Charles

[cwbaker]

Fixed by @elushaX.