Skip to content
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.

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

Fix compilation under and enable C++20 #12658

Merged
1 commit merged into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 107 additions & 69 deletions dep/CLI11/CLI11.hpp

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions dep/CLI11/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# CLI11

Taken from [release v1.9.0](https://github.com/CLIUtils/CLI11/releases/tag/v1.9.0), source commit
[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)
Comment on lines +3 to +4
Copy link
Member Author

@lhecker lhecker Mar 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

2 changes: 1 addition & 1 deletion src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ namespace SettingsModelLocalTests

for (size_t i = 0; i < expectedCampbellTable.size(); i++)
{
const auto& expected = expectedCampbellTable.at(i);
const til::color expected{ expectedCampbellTable.at(i) };
const til::color actual{ scheme->Table().at(static_cast<uint32_t>(i)) };
VERIFY_ARE_EQUAL(expected, actual);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,11 +795,11 @@ namespace SettingsModelLocalTests
const auto terminalSettings4 = createTerminalSettings(activeProfiles.GetAt(4), colorSchemes);
const auto terminalSettings5 = createTerminalSettings(activeProfiles.GetAt(5), colorSchemes);

VERIFY_ARE_EQUAL(RGB(0x12, 0x34, 0x56), terminalSettings0->CursorColor()); // from color scheme
VERIFY_ARE_EQUAL(til::color(0x12, 0x34, 0x56), terminalSettings0->CursorColor()); // from color scheme
VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings1->CursorColor()); // default
VERIFY_ARE_EQUAL(RGB(0x23, 0x45, 0x67), terminalSettings2->CursorColor()); // from profile (trumps color scheme)
VERIFY_ARE_EQUAL(RGB(0x34, 0x56, 0x78), terminalSettings3->CursorColor()); // from profile (not set in color scheme)
VERIFY_ARE_EQUAL(RGB(0x45, 0x67, 0x89), terminalSettings4->CursorColor()); // from profile (no color scheme)
VERIFY_ARE_EQUAL(til::color(0x23, 0x45, 0x67), terminalSettings2->CursorColor()); // from profile (trumps color scheme)
VERIFY_ARE_EQUAL(til::color(0x34, 0x56, 0x78), terminalSettings3->CursorColor()); // from profile (not set in color scheme)
VERIFY_ARE_EQUAL(til::color(0x45, 0x67, 0x89), terminalSettings4->CursorColor()); // from profile (no color scheme)
VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings5->CursorColor()); // default
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ namespace winrt::TerminalApp::implementation
const auto package{ GetCurrentPackageNoThrow() };
if (package == nullptr)
{
return;
co_return;
}

const auto tryEnableStartupTask = _settings.GlobalSettings().StartOnUserLogin();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TabBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ namespace winrt::TerminalApp::implementation

if (_keyChord == keyChordText)
{
return;
co_return;
}

_keyChord = keyChordText;
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ namespace winrt::TerminalApp::implementation
// Don't reload our icon if it hasn't changed.
if (iconPath == _lastIconPath)
{
return;
co_return;
}

_lastIconPath = iconPath;
Expand All @@ -283,7 +283,7 @@ namespace winrt::TerminalApp::implementation
// for when we show the icon again)
if (_iconHidden)
{
return;
co_return;
}

auto weakThis{ get_weak() };
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2274,7 +2274,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
if (_IsClosing())
{
return;
co_return;
}

if (e.DataView().Contains(StandardDataFormats::ApplicationLink()))
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/FileUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. 🥲

static constexpr std::wstring_view UnpackagedSettingsFolderName{ L"Microsoft\\Windows Terminal\\" };

namespace winrt::Microsoft::Terminal::Settings::Model
Expand Down
3 changes: 2 additions & 1 deletion src/common.build.pre.props
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@
<RuntimeTypeInfo>false</RuntimeTypeInfo>
<ConformanceMode>true</ConformanceMode>
<UseStandardPreprocessor>true</UseStandardPreprocessor>
<LanguageStandard>stdcpp17</LanguageStandard>
<LanguageStandard Condition="$(MSBuildVersion) &lt; '17.0.0'">stdcpp17</LanguageStandard>
<LanguageStandard Condition="$(MSBuildVersion) &gt;= '17.0.0'">stdcpp20</LanguageStandard>
<LanguageStandard_C>stdc17</LanguageStandard_C>
<AdditionalOptions>%(AdditionalOptions) /utf-8 /Zc:externConstexpr /Zc:lambda /Zc:throwingNew</AdditionalOptions>
<ControlFlowGuard>Guard</ControlFlowGuard>
Expand Down
2 changes: 1 addition & 1 deletion src/host/ft_host/InitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static FILE* std_in = nullptr;
// This will automatically try to terminate the job object (and all of the
// binaries under test that are children) whenever this class gets shut down.
// also closes the FILE pointers created by reopening stdin and stdout.
auto OnAppExitKillJob = wil::scope_exit([&] {
auto OnAppExitKillJob = wil::scope_exit([] {
if (std_out != nullptr)
{
fclose(std_out);
Expand Down
4 changes: 1 addition & 3 deletions src/inc/til/bit.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ namespace til
template<class To, class From, std::enable_if_t<std::conjunction_v<std::bool_constant<sizeof(To) == sizeof(From)>, std::is_trivially_copyable<To>, std::is_trivially_copyable<From>>, int> = 0>
[[nodiscard]] constexpr To bit_cast(const From& _Val) noexcept
{
#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
Copy link
Member Author

@lhecker lhecker Mar 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

return __builtin_bit_cast(To, _Val);
}
}
15 changes: 8 additions & 7 deletions src/inc/til/enumset.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
template<typename... Args>
static constexpr UnderlyingType to_underlying(Args... positions) noexcept
{
return ((UnderlyingType{ 1 } << static_cast<UnderlyingType>(positions)) | ...);
}

template<>
static constexpr UnderlyingType to_underlying() noexcept
{
return 0;
if constexpr (sizeof...(positions) == 0)
{
return 0;
}
else
{
return ((UnderlyingType{ 1 } << static_cast<UnderlyingType>(positions)) | ...);
}
}

UnderlyingType _data{};
Expand Down
36 changes: 19 additions & 17 deletions src/terminal/parser/ft_fuzzer/fuzzing_directed.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,25 +253,27 @@ namespace fuzz
template<class _Type>
static _Type GetRandom(__in _Type tMin, __in _Type tMax)
{
std::mt19937 engine(m_rd()); // Mersenne twister MT19937
std::uniform_int_distribution<_Type> distribution(tMin, tMax);
auto generator = std::bind(distribution, engine);
return generator();
if constexpr (std::is_same_v<_Type, BYTE>)
{
// uniform_int_distribution only works with _Is_IntType types, which do not
// currently include char or unsigned char, so here is a specialization
// specifically for BYTE (unsigned char).
std::mt19937 engine(m_rd()); // Mersenne twister MT19937
// BYTE is unsigned, so we want to also use an unsigned type to avoid sign
// extension of tMin and tMax.
std::uniform_int_distribution<unsigned short> distribution(tMin, tMax);
auto generator = std::bind(distribution, engine);
return static_cast<BYTE>(generator());
}
else
{
std::mt19937 engine(m_rd()); // Mersenne twister MT19937
std::uniform_int_distribution<_Type> distribution(tMin, tMax);
auto generator = std::bind(distribution, engine);
return generator();
}
}

// uniform_int_distribution only works with _Is_IntType types, which do not
// currently include char or unsigned char, so here is a specialization
// specifically for BYTE (unsigned char).
template<>
static BYTE GetRandom(__in BYTE tMin, __in BYTE tMax)
{
std::mt19937 engine(m_rd()); // Mersenne twister MT19937
// BYTE is unsigned, so we want to also use an unsigned type to avoid sign
// extension of tMin and tMax.
std::uniform_int_distribution<unsigned short> distribution(tMin, tMax);
auto generator = std::bind(distribution, engine);
return static_cast<BYTE>(generator());
}
#ifdef __min_collision__
#undef __min_collision__
#define min(a, b) (((a) < (b)) ? (a) : (b))
Expand Down