Building for Windows using C++20 #7841

kneth · 2024-06-26T10:43:13Z

What, How & Why?

This PR tries to address the following:

Recently Core has been change to leave the decision about the C++ standard to the SDKs
Bindgen generates code with requires which is new keyword in C++20: node template and jsi template
Core's file operations are using u8string() on Windows, and signature has changed between C++17 and C++20 with the implication that Core only supports C++17 on Windows

☑️ ToDos

📝 Changelog update
🚦 Tests (or not relevant)
~~[ ] C-API, if public C++ API changed~~
~~[ ] bindgen/spec.yml, if public C++ API changed~~

coveralls-official · 2024-06-26T11:20:06Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_16

Details

0 of 0 changed or added relevant lines in 0 files are covered.
90 unchanged lines in 17 files lost coverage.
Overall coverage decreased (-0.02%) to 90.961%

Files with Coverage Reduction	New Missed Lines	%
src/realm/array_string.cpp	1	87.23%
src/realm/util/file.cpp	1	85.3%
test/test_index_string.cpp	1	93.48%
test/test_query2.cpp	1	98.73%
src/realm/table_view.cpp	2	92.99%
test/object-store/util/test_file.cpp	2	86.29%
test/test_all.cpp	2	75.82%
src/realm/unicode.cpp	3	83.83%
src/realm/sync/noinst/client_impl_base.cpp	4	82.56%
src/realm/sync/noinst/server/server.cpp	5	73.82%

Totals
Change from base Build 2434:	-0.02%
Covered Lines:	214709
Relevant Lines:	236046

💛 - Coveralls

coveralls-official · 2024-06-26T14:17:20Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_18

Details

0 of 0 changed or added relevant lines in 0 files are covered.
107 unchanged lines in 19 files lost coverage.
Overall coverage decreased (-0.03%) to 90.943%

Files with Coverage Reduction	New Missed Lines	%
src/realm/query_engine.hpp	1	94.15%
src/realm/util/file.cpp	1	84.84%
src/realm/util/serializer.cpp	1	90.43%
test/fuzz_tester.hpp	1	57.73%
test/test_query2.cpp	1	98.73%
src/realm/object-store/shared_realm.cpp	2	91.89%
src/realm/table_view.cpp	2	92.99%
test/test_all.cpp	2	75.82%
src/realm/mixed.cpp	3	86.46%
src/realm/query_expression.hpp	3	93.84%

Totals
Change from base Build 2434:	-0.03%
Covered Lines:	214668
Relevant Lines:	236046

💛 - Coveralls

coveralls-official · 2024-06-26T15:29:43Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_19

Details

0 of 0 changed or added relevant lines in 0 files are covered.
81 unchanged lines in 18 files lost coverage.
Overall coverage increased (+0.008%) to 90.985%

Files with Coverage Reduction	New Missed Lines	%
src/realm/array_string.cpp	1	87.23%
src/realm/sync/noinst/server/server_history.cpp	1	63.7%
src/realm/util/file.cpp	1	84.84%
src/realm/util/serializer.cpp	1	90.43%
test/fuzz_tester.hpp	1	57.73%
test/test_index_string.cpp	1	93.48%
test/test_query2.cpp	1	98.73%
src/realm/sync/noinst/server/server.cpp	2	73.78%
test/test_all.cpp	2	75.82%
src/realm/mixed.cpp	3	86.46%

Totals
Change from base Build 2434:	0.008%
Covered Lines:	214771
Relevant Lines:	236052

💛 - Coveralls

ironage

If we are officially supporting C++20, then we should have a CI runner that checks for compile errors so that we don't accidentally regress.

ironage · 2024-06-27T18:48:41Z

src/realm/util/file.cpp

+}
+inline auto u8path(const std::string& str)
+{
+    return std::filesystem::path(reinterpret_cast<const char8_t*>(str.c_str()));


I was a bit concerned that this converts using the constructor that terminates at the first null byte, regardless of the length of the original string. But I suppose this is not actually a problem because null bytes cannot be in a file path.

jedelbo · 2024-07-02T12:30:18Z

@ironage I tried to get it to compile on ubuntu2204-arm64-large, but it failed with both gcc-11 and clang-18. I think clang-18 needs libstdc++-12 to work. On my machine it fails with gcc-11 (probably too old), but succeeds with gcc-12 and clang-16.

coveralls-official · 2024-07-02T13:07:50Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_25

Details

0 of 0 changed or added relevant lines in 0 files are covered.
56 unchanged lines in 11 files lost coverage.
Overall coverage decreased (-0.003%) to 90.971%

Files with Coverage Reduction	New Missed Lines	%
src/realm/dictionary.cpp	1	85.16%
src/realm/query_expression.hpp	1	93.85%
src/realm/cluster.cpp	2	75.85%
test/object-store/sync/flx_sync.cpp	2	98.27%
test/object-store/util/test_file.cpp	2	86.29%
test/test_lang_bind_helper.cpp	2	93.2%
src/realm/collection_parent.cpp	3	93.08%
src/realm/sync/network/websocket.cpp	5	71.21%
src/realm/bplustree.cpp	6	72.55%
src/realm/sync/noinst/server/server.cpp	8	74.29%

Totals
Change from base Build 2466:	-0.003%
Covered Lines:	215115
Relevant Lines:	236466

💛 - Coveralls

jedelbo · 2024-07-02T13:55:57Z

Trying to get it to build on Windows also failed (works on my machine)

coveralls-official · 2024-07-02T14:36:20Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_28

Details

0 of 0 changed or added relevant lines in 0 files are covered.
50 unchanged lines in 13 files lost coverage.
Overall coverage increased (+0.01%) to 90.986%

Files with Coverage Reduction	New Missed Lines	%
src/realm/dictionary.cpp	1	85.16%
src/realm/query_expression.hpp	1	93.81%
src/realm/sort_descriptor.cpp	1	94.06%
test/test_index_string.cpp	1	93.48%
src/realm/object-store/shared_realm.cpp	2	91.89%
src/realm/query_expression.cpp	2	86.62%
src/realm/sync/network/http.hpp	2	82.27%
src/realm/table.cpp	3	90.42%
src/realm/unicode.cpp	3	83.83%
test/fuzz_group.cpp	6	44.98%

Totals
Change from base Build 2466:	0.01%
Covered Lines:	215158
Relevant Lines:	236473

💛 - Coveralls

ironage · 2024-07-03T22:58:38Z

Building with -D CMAKE_CXX_STANDARD=20 works on evergreen's ubuntu 24.04 builders; they use gcc 13.2.0. I think it should be sufficient to add this to the builder I just added in #7856. What do you think?

kneth · 2024-07-04T14:12:20Z

@ironage

evergreen's ubuntu 24.04 builders

I am trying in ff889df

coveralls-official · 2024-07-04T15:28:23Z

Pull Request Test Coverage Report for Build kenneth.geisshirt_30

Details

0 of 0 changed or added relevant lines in 0 files are covered.
31 unchanged lines in 13 files lost coverage.
Overall coverage increased (+0.03%) to 90.986%

Files with Coverage Reduction	New Missed Lines	%
src/realm/dictionary.cpp	1	85.16%
src/realm/object-store/sync/sync_session.cpp	1	92.01%
src/realm/sort_descriptor.cpp	1	94.06%
src/realm/sync/network/websocket.cpp	1	71.79%
src/realm/sync/noinst/client_impl_base.cpp	1	82.1%
test/fuzz_tester.hpp	1	57.73%
src/realm/array_blobs_big.cpp	2	98.58%
src/realm/cluster.cpp	2	75.85%
src/realm/query_expression.hpp	2	93.81%
src/realm/sync/client.cpp	3	90.98%

Totals
Change from base Build 2469:	0.03%
Covered Lines:	215219
Relevant Lines:	236542

💛 - Coveralls

nielsenko

LGTM. Tried running the Dart test suite with this commit with set(CMAKE_CXX_STANDARD 20) without any hickups.

kneth · 2024-07-05T09:07:33Z

Builds for node.js for Windows: realm/realm-js#6739

cla-bot bot added the cla: yes label Jun 26, 2024

github-actions bot assigned kneth Jun 26, 2024

jedelbo force-pushed the kneth/cpp20-windows branch from 53a760d to 459ef83 Compare June 26, 2024 13:27

jedelbo marked this pull request as ready for review June 27, 2024 07:18

jedelbo requested review from tgoyne and ironage June 27, 2024 07:18

ironage reviewed Jun 27, 2024

View reviewed changes

tgoyne approved these changes Jul 1, 2024

View reviewed changes

jedelbo force-pushed the kneth/cpp20-windows branch 4 times, most recently from 6c5cebf to e923527 Compare July 2, 2024 12:26

jedelbo force-pushed the kneth/cpp20-windows branch from e923527 to 997f636 Compare July 2, 2024 12:32

jedelbo force-pushed the kneth/cpp20-windows branch 3 times, most recently from edc77ab to aedf6d3 Compare July 2, 2024 13:55

kneth and others added 5 commits July 4, 2024 16:05

Building using C++20 on Windows

40a43ad

Enable __cplusplus

7a1ddd6

if and not ifdef

0e451e1

Improvements

137d911

Fix building with gcc and clang

ade465f

kneth force-pushed the kneth/cpp20-windows branch from aedf6d3 to ade465f Compare July 4, 2024 14:05

Building on Linux with C++20

ff889df

ironage approved these changes Jul 4, 2024

View reviewed changes

nielsenko approved these changes Jul 4, 2024

View reviewed changes

kneth added the no-jira-ticket Skip checking the PR title for Jira reference label Jul 5, 2024

kneth merged commit 8259932 into master Jul 5, 2024
43 of 44 checks passed

kneth deleted the kneth/cpp20-windows branch July 5, 2024 09:08

github-actions bot mentioned this pull request Jul 5, 2024

Prepare for 14.10.3 #7865

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Building for Windows using C++20 #7841

Building for Windows using C++20 #7841

kneth commented Jun 26, 2024

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

ironage left a comment

ironage Jun 27, 2024

jedelbo commented Jul 2, 2024 •

edited

Loading

coveralls-official bot commented Jul 2, 2024 •

edited

Loading

jedelbo commented Jul 2, 2024

coveralls-official bot commented Jul 2, 2024 •

edited

Loading

ironage commented Jul 3, 2024

kneth commented Jul 4, 2024

coveralls-official bot commented Jul 4, 2024 •

edited

Loading

nielsenko left a comment

kneth commented Jul 5, 2024

Building for Windows using C++20 #7841

Building for Windows using C++20 #7841

Conversation

kneth commented Jun 26, 2024

What, How & Why?

☑️ ToDos

coveralls-official bot commented Jun 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_16

Details

💛 - Coveralls

coveralls-official bot commented Jun 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_18

Details

💛 - Coveralls

coveralls-official bot commented Jun 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_19

Details

💛 - Coveralls

ironage left a comment

Choose a reason for hiding this comment

ironage Jun 27, 2024

Choose a reason for hiding this comment

jedelbo commented Jul 2, 2024 • edited Loading

coveralls-official bot commented Jul 2, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_25

Details

💛 - Coveralls

jedelbo commented Jul 2, 2024

coveralls-official bot commented Jul 2, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_28

Details

💛 - Coveralls

ironage commented Jul 3, 2024

kneth commented Jul 4, 2024

coveralls-official bot commented Jul 4, 2024 • edited Loading

Pull Request Test Coverage Report for Build kenneth.geisshirt_30

Details

💛 - Coveralls

nielsenko left a comment

Choose a reason for hiding this comment

kneth commented Jul 5, 2024

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

coveralls-official bot commented Jun 26, 2024 •

edited

Loading

jedelbo commented Jul 2, 2024 •

edited

Loading

coveralls-official bot commented Jul 2, 2024 •

edited

Loading

coveralls-official bot commented Jul 2, 2024 •

edited

Loading

coveralls-official bot commented Jul 4, 2024 •

edited

Loading