Fix compilation under and enable C++20 #12658

lhecker · 2022-03-10T15:46:24Z

This commit enables /std:c++20 for local development under VS17.
Our CIs will continue to use VS16 and C++17 for now in order to reduce
the likelihood of regressions during the current development cycle.
It's expected that we'll migrate to VS17 soon, as this
is what conhost is already being built with anyways.

PR Checklist

Closes [MSVC][std:c++latest] terminal failed to build with /std:c++latest on MSVC #12510
I work here
Tests added/passed

Validation Steps Performed

Everything compiles under /std:c++20 ✅

lhecker · 2022-03-10T15:48:17Z

dep/CLI11/README.md

+Taken from [release v1.9.1](https://github.com/CLIUtils/CLI11/releases/tag/v1.9.1), source commit
+[5cb3efa](https://github.com/CLIUtils/CLI11/commit/5cb3efabce007c3a0230e4cc2e27da491c646b6c)


FYI In order to keep this PR compact I manually modified CLI11.hpp.
First I reformatted it with clang-format and then went through the file, reverting all irrelevant whitespace changes.

Without this basically the entire CLI11.hpp file would've been rewritten, since the original v1.9.0 uses a vastly different code format than the version we got.

lhecker · 2022-03-10T15:48:42Z

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -12,7 +12,7 @@
 #include <sddl.h>
 #include <wil/token_helpers.h>

-static constexpr std::string_view Utf8Bom{ u8"\uFEFF" };
+static constexpr std::string_view Utf8Bom{ "\xEF\xBB\xBF", 3 };


https://en.wikipedia.org/wiki/Byte_order_mark#UTF-8

did we lose support for u8 literals? was that not a real c++ feature?

Yea what's the deal here? was this just a happy accident that the compiler would translate \uFEFF to the utf8 \xEF\xBB\xBF, and now we're just being more explicit?

This is a fallout from a breaking change in C++20 (source):

u8"s-char-sequence(optional)"
UTF-8 string literal. The type of a u8"..." string literal is const char[N] (until C++20) const char8_t[N] (since C++20), where N is the size of the string in UTF-8 code units including the null terminator.

Breaking changes in the language; still no breaking ABI change in the STL. 🥲

lhecker · 2022-03-10T15:51:11Z

src/inc/til/bit.h

-#ifdef __cpp_lib_bit_cast
-#warning "Replace til::bit_cast and __builtin_bit_cast with std::bit_cast"
-#endif
+        // TODO: Replace til::bit_cast and __builtin_bit_cast with std::bit_cast


If you'd like me to I can create an issue for this. I left it as a code-TODO since I personally consider this not particularly noteworthy. (I considered doing it as part of this PR, but I didn't want to burden us any more than necessary.)

src/terminal/parser/ft_fuzzer/fuzzing_directed.h

DHowett · 2022-03-10T18:57:37Z

dep/CLI11/README.md

-[dd0d8e4](https://github.com/CLIUtils/CLI11/commit/dd0d8e4fe729e5b1110232c7a5c9566dad884686)
-
+Taken from [release v1.9.1](https://github.com/CLIUtils/CLI11/releases/tag/v1.9.1), source commit
+[5cb3efa](https://github.com/CLIUtils/CLI11/commit/5cb3efabce007c3a0230e4cc2e27da491c646b6c)


if there is a cgmanifest.json in the cli11 folder plz update it to match this commit

DHowett · 2022-03-10T18:58:15Z

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -12,7 +12,7 @@
 #include <sddl.h>
 #include <wil/token_helpers.h>

-static constexpr std::string_view Utf8Bom{ u8"\uFEFF" };
+static constexpr std::string_view Utf8Bom{ "\xEF\xBB\xBF", 3 };


did we lose support for u8 literals? was that not a real c++ feature?

zadjii-msft · 2022-03-10T22:46:52Z

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -12,7 +12,7 @@
 #include <sddl.h>
 #include <wil/token_helpers.h>

-static constexpr std::string_view Utf8Bom{ u8"\uFEFF" };
+static constexpr std::string_view Utf8Bom{ "\xEF\xBB\xBF", 3 };


Yea what's the deal here? was this just a happy accident that the compiler would translate \uFEFF to the utf8 \xEF\xBB\xBF, and now we're just being more explicit?

ghost · 2022-03-10T23:28:06Z

Hello @lhecker!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 18 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

When Leonard updated CLI11 in #12658, he imported a version that no longer requires RTTI. Since CLI11 was the only reason we had enabled RTTI in any of our projects, we're finally able to turn it off. | | TerminalApp.dll | MSIX Package | | ------ | ---------------:| ------------:| | Before | 3180KiB | 6545KiB | | After | 1970KiB | 6426KiB | | Delta | **-1210KiB** | **-119KiB** |

When Leonard updated CLI11 in #12658, he imported a version that no longer requires RTTI. Since CLI11 was the only reason we had enabled RTTI in any of our projects, we're finally able to turn it off. | | TerminalApp.dll | MSIX Package | | ------ | ---------------:| ------------:| | Before | 3180KiB | 6545KiB | | After | 1970KiB | 6426KiB | | Delta | **-1210KiB** | **-119KiB** | (cherry picked from commit e682cda) Service-Card-Id: 85546966 Service-Version: 1.15

ghost added Area-Build Issues pertaining to the build system, CI, infrastructure, meta Issue-Task It's a feature request, but it doesn't really need a major design. Priority-1 A description (P1) Product-Meta The product is the management of the products. labels Mar 10, 2022

lhecker commented Mar 10, 2022

View reviewed changes

Fix compilation under and enable C++20

e4003ae

lhecker force-pushed the dev/lhecker/issue-12510-cpp20 branch from 4f9861c to e4003ae Compare March 10, 2022 17:12

DHowett approved these changes Mar 10, 2022

View reviewed changes

zadjii-msft approved these changes Mar 10, 2022

View reviewed changes

lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 10, 2022

ghost merged commit cfaa315 into main Mar 10, 2022

ghost deleted the dev/lhecker/issue-12510-cpp20 branch March 10, 2022 23:47

DHowett mentioned this pull request Sep 8, 2022

Disable RTTI on TerminalApp and friends #13947

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix compilation under and enable C++20 #12658

Fix compilation under and enable C++20 #12658

lhecker commented Mar 10, 2022

lhecker Mar 10, 2022 •

edited

Loading

lhecker Mar 10, 2022

DHowett Mar 10, 2022

zadjii-msft Mar 10, 2022

lhecker Mar 10, 2022

lhecker Mar 10, 2022 •

edited

Loading

DHowett Mar 10, 2022

DHowett Mar 10, 2022

zadjii-msft Mar 10, 2022

ghost commented Mar 10, 2022

		Taken from [release v1.9.1](https://github.com/CLIUtils/CLI11/releases/tag/v1.9.1), source commit
		[5cb3efa](https://github.com/CLIUtils/CLI11/commit/5cb3efabce007c3a0230e4cc2e27da491c646b6c)

Fix compilation under and enable C++20 #12658

Fix compilation under and enable C++20 #12658

Conversation

lhecker commented Mar 10, 2022

PR Checklist

Validation Steps Performed

lhecker Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

lhecker Mar 10, 2022

Choose a reason for hiding this comment

DHowett Mar 10, 2022

Choose a reason for hiding this comment

zadjii-msft Mar 10, 2022

Choose a reason for hiding this comment

lhecker Mar 10, 2022

Choose a reason for hiding this comment

lhecker Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

DHowett Mar 10, 2022

Choose a reason for hiding this comment

DHowett Mar 10, 2022

Choose a reason for hiding this comment

zadjii-msft Mar 10, 2022

Choose a reason for hiding this comment

ghost commented Mar 10, 2022

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

lhecker Mar 10, 2022 •

edited

Loading

lhecker Mar 10, 2022 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.