Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] Support for MSVC #1236

Closed
3 of 11 tasks
jfalcou opened this issue Feb 10, 2022 · 16 comments
Closed
3 of 11 tasks

[FEATURE] Support for MSVC #1236

jfalcou opened this issue Feb 10, 2022 · 16 comments
Labels
c++20 C++20 transition feature New feature or request in-progress Works is being done on this issue [PROJECT] Overarchign set of tasks

Comments

@jfalcou
Copy link
Owner

jfalcou commented Feb 10, 2022 •
edited
Loading

This is a mutable, ongoing issue to discuss/list steps for proper MSVC support.

Current Status

Since MSVC 2022 19.30, C++20 support has drastically improved. Building blocks libraries like kumi and raberu now natively compile on MSVC without any hassle. This has triggered the investigation of how much EVE needs to be fixed to make MSVC EVE a thing.

Note that we currently don't care about code generation quality as our first goal here is to get the library to compile.

Plan of Actions

We need to use our module based tests system to validate EVE code on MSVC independant piece after the next.
This is the ordered list of module to make pass:

  • unit.api.exe
  • unit.memory.exe
  • unit.core.exe
  • unit.math.exe
  • unit.algo.exe
  • unit.views.exe
  • All other modules
  • doc.exe

Best process so far:

  • Hack away on MSVC then push to validate CI.
  • If anything break on g++/clang, fix accordingly then rince & repeat.
  • Prefer code reorganisation/rewrite to compiler specific #if #else #endif whenever possible

MSVC also produces a lot of obnoxious warning for conversions. Some are really stupid IMHO but try to fix them
as best as we can unless it makes the code ugly.

Current Status

  • unit.arch.exe compiles, pass tests and validate CI
  • unit.meta.exe compiles and pass tests and validate CI
  • unit.internals.exe compiles and pass tests and validate CI
@jfalcou jfalcou added feature New feature or request in-progress Works is being done on this issue c++20 C++20 transition [PROJECT] Overarchign set of tasks labels Feb 10, 2022
@DenisYaroshevskiy
Copy link
Collaborator

CI?

I'll have to install windows somewhere, won't I?

@jfalcou
Copy link
Owner Author

jfalcou commented Feb 10, 2022

Was thinking of using appveyor

jfalcou added a commit that referenced this issue Feb 10, 2022
@jfalcou
Issue #1236 - First batch of fixes for MSVC support
27561a6
@Tradias
Copy link

Tradias commented Nov 26, 2022

Is there anything you need help with here?

Apart from appveyor, I have only ever used Github Actions and they were easy to set up.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

I just need to find time to tackle the stupid errors that prevent other test to compile.

Appveyor is setup already.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022 •
edited
Loading

It looks like some compile time traits we have don't fly properly but the error messages are outrageous.

I will push a more up-to-date list of breaking thing later today.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

So mostly we need help understanding why msvc don't understand our properly standard templatr monstrosity

@Tradias
Copy link

Tradias commented Nov 26, 2022

Sure, sounds like something I can investigate.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

Good. Let me update this issue later withore info

@jfalcou jfalcou pinned this issue Nov 26, 2022
@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

@Tradias added a small roadmap on how to tackle the thing. Tell me if you need help.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

unit.api targets

  • Looks like we try to use constepxr value as lambda capture in some of the shuffle tests and MSVC doesn't appreciate that

@Tradias
Copy link

Tradias commented Nov 26, 2022 •
edited
Loading

Looks good, I have managed to get unit.core.exe to compile without significant code changes. Creating a pr now.

@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022 •
edited
Loading

Good job wow :D
Did you used the in flight PR that fixes unit.internals (#1458) ? or were the issue unrelated ?

@Tradias Tradias mentioned this issue Nov 26, 2022
Closed
@Tradias
Copy link

Tradias commented Nov 26, 2022

I didn't know about #1458, one of the issue was in fact related. I think my solution might be faster to compile but potentially incorrect.

DenisYaroshevskiy pushed a commit that referenced this issue Nov 26, 2022
@jfalcou
#1236 - unit.internals.exe compiles on latest MSVC (#1458)
19922ed 
* Simplify code around find_pattern

* Fix very vocal warning

* Simplify value_type_t

* unit.internals now compiles

* Fix over-eager value_type fix
@jfalcou
Copy link
Owner Author

jfalcou commented Nov 26, 2022

I had the same code at one point but it failed on other compilers. Maybe we can borrow your cocnept based strategy but lay it out like ours ?

@Tradias
Copy link

Tradias commented Nov 26, 2022

Yes that is possible. I have reverted it to your version for now.

jfalcou added a commit that referenced this issue Nov 27, 2022
@jfalcou
#1236 - Small adaptation for MSVC test running
d9c58bc
@jfalcou
Copy link
Owner Author

jfalcou commented Nov 27, 2022

Merged the CI adaptation to see if it is faster to compiles

@jfalcou jfalcou unpinned this issue Feb 9, 2023
Repository owner locked and limited conversation to collaborators Feb 9, 2023
@jfalcou jfalcou converted this issue into discussion #1550 Feb 9, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Assignees
No one assigned
Labels
c++20 C++20 transition feature New feature or request in-progress Works is being done on this issue [PROJECT] Overarchign set of tasks
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jfalcou @Tradias @DenisYaroshevskiy