Feat/286/std expected

[jowillianto]

No description provided.

[liuzicheng1987]

Hi @jowillianto,

Thanks for the PR. I look forward to having this in the library.

However, the tests fail to compile. Should be an easy fix, though. Could you take a look?

[jowillianto]

I will try my best to fix these for linux and macos first. My main machine is a macbook pro. In the end, if I cannot fix for windows, please help me.

[liuzicheng1987]

@jowillianto , I will take a look tonight

[liuzicheng1987]

@jowillianto, the tests and benchmarks now compile and run through.

[jowillianto]

Thanks. I was setting up environments.

[jowillianto]

So what should we do now? @liuzicheng1987

[liuzicheng1987]

@jowillianto, I think there are two more things to do:

1. We need a formal testing pipeline for C++-23.
2. We need to remove the result type from all .src files and move them into the headers files, otherwise users might experience confusing linking errors.

[jowillianto]

I think running the tests by compiling it in both cpp23 and cpp20 should serve the purpose. Additionally, I think we should compile while projects in the same cpp version otherwise linking errors or what I experience is everything works but incorrect behaviour.

[liuzicheng1987]

@jowillianto , I dug a bit deeper and found that a lot of the weird compilation and runtime errors we have been getting is that the compiler was mixing the old result type with std::expected. Because a "Result" wasn't the same across different compilation units we were getting all kinds of weird errors. I have found that designing it as an explicit opt-in resolves at least some of the issues.

[liuzicheng1987]

@jowillianto, you said your main machine is a MacBook Pro. I wonder, how did you compile it? I cannot get this to compile with Clang. It appears they don't fully support std::expected yet, so now I only test for GCC.

[jowillianto]

@liuzicheng1987
I have used llvm-17 to compile reflect-cpp up until now, I have tried compiling with llvm-18 and llvm-19. Up until now, llvm-17 seems to be the most stable with the least abi bugs. llvm-18 have a problematic clangd that complains about abi bugs all the time. While, llvm-19 sometimes does not compile the code.

[liuzicheng1987]

@jowillianto , I can compile the library and the tests locally using Clang-19 without any issues. The tests pass. However, the Github Actions build pipeline does not seem to be ready for Clang-19 quite yet.

That being said, I think this is ready to be merged. Do you agree?

Thank you so much for contributing this.

[jowillianto]

I will look at this one last time tomorrow. I'll leave a comment when I am done. :)

[jowillianto]

I added a version switch for CXX20 and CXX23 in CMakeLists.txt because it apparently does not compile without specifying any options. @liuzicheng1987. Otherwise, the tests should have covered most of the things.

[liuzicheng1987]

@jowillianto, I see...I actually updated all relevant parts of the documentation to explicitly include definitions of the standard:

0533312

I think this is probably the better solution anyway, because it allows people to specify the standard they want. But your solution is good as well. Should I revert the relevant changes to the documentation or should we simply revert your last commit?

[liuzicheng1987]

@jowillianto, another solution would be something of the order "If the standard is not explicitly defined by the user, then use 20 or 23."

[jowillianto]

This should be good to go now !

[liuzicheng1987]

I'll just wait for the tests to run through, just as a matter of formality, and then I will merge.

[liuzicheng1987]

@jowillianto , thank you so much for your contribution.

[liuzicheng1987]

@jowillianto , sorry, fat fingers...

[jowillianto]

Likewise, it is a good experience working with repository this big. This is probably my first time. However, I really like the modern functional-style design principles. I hope more C++ code can have such modern design principles.