Enable /Zc:twoPhase

src/cascadia/TerminalControl/pch.h

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 //
 // pch.h
@@ -58,6 +58,7 @@ TRACELOGGING_DECLARE_PROVIDER(g_hTerminalControlProvider);
 #include <telemetry/ProjectTelemetry.h>
 
 #include <WinUser.h>
+#include <ShlObj_core.h>
 
 #include "til.h"
 

src/cascadia/TerminalSettingsModel/ActionAndArgs.h

@@ -5,7 +5,6 @@
 
 #include "ActionAndArgs.g.h"
 #include "ActionArgs.h"
-#include "TerminalWarnings.h"
 #include "../inc/cppwinrt_utils.h"
 
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation
@@ -36,6 +35,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
     BASIC_FACTORY(ActionAndArgs);
 }
 
+#ifdef JSON_UTILS_H
+#error foo
+#endif
+
 namespace Microsoft::Terminal::Settings::Model::JsonUtils
 {
     using namespace winrt::Microsoft::Terminal::Settings::Model;

src/cascadia/TerminalSettingsModel/ActionArgs.cpp

@@ -40,6 +40,9 @@
 #include <LibraryResources.h>
 #include <WtExeUtils.h>
 
+#include "TerminalSettingsSerializationHelpers.h"
+#include "JsonUtils.h"
+
 using namespace winrt::Microsoft::Terminal::Control;
 
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation

src/cascadia/TerminalSettingsModel/ActionArgs.h

@@ -40,12 +40,10 @@
 #include "MultipleActionsArgs.g.h"
 
 #include "../../cascadia/inc/cppwinrt_utils.h"
-#include "JsonUtils.h"
-#include "HashUtils.h"
 #include "TerminalWarnings.h"
 #include "../inc/WindowingBehavior.h"
 
-#include "TerminalSettingsSerializationHelpers.h"
+#include "HashUtils.h"
 
 #define ACTION_ARG(type, name, ...)                                                                    \
 public:                                                                                                \

src/cascadia/TerminalSettingsModel/AppearanceConfig.h

@@ -17,7 +17,6 @@ Author(s):
 #pragma once
 
 #include "AppearanceConfig.g.h"
-#include "JsonUtils.h"
 #include "IInheritable.h"
 #include "MTSMSettings.h"
 #include <DefaultSettings.h>

src/cascadia/TerminalSettingsModel/FontConfig.h

@@ -18,7 +18,6 @@ Author(s):
 
 #include "pch.h"
 #include "FontConfig.g.h"
-#include "JsonUtils.h"
 #include "MTSMSettings.h"
 #include "../inc/cppwinrt_utils.h"
 #include "IInheritable.h"

src/cascadia/TerminalSettingsModel/IconPathConverter.cpp

@@ -67,7 +67,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             try
             {
                 winrt::Windows::Foundation::Uri iconUri{ path };
-                BitmapIconSource<TIconSource>::type iconSource;
+                typename BitmapIconSource<TIconSource>::type iconSource;
                 // Make sure to set this to false, so we keep the RGB data of the
                 // image. Otherwise, the icon will be white for all the
                 // non-transparent pixels in the image.
@@ -81,6 +81,16 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         return nullptr;
     }
 
+    static winrt::hstring _expandIconPath(hstring iconPath)
+    {
+        if (iconPath.empty())
+        {
+            return iconPath;
+        }
+        winrt::hstring envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(iconPath.c_str()) };
+        return envExpandedPath;
+    }
+
     // Method Description:
     // - Creates an IconSource for the given path.
     //    * If the icon is a path to an image, we'll use that.
@@ -115,7 +125,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             {
                 try
                 {
-                    FontIconSource<TIconSource>::type icon;
+                    typename FontIconSource<TIconSource>::type icon;
                     const wchar_t ch = iconPath[0];
 
                     // The range of MDL2 Icons isn't explicitly defined, but
@@ -146,7 +156,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             // Swapping between nullptr IconSources and non-null IconSources causes a crash
             // to occur, but swapping between IconSources with a null source and non-null IconSources
             // work perfectly fine :shrug:.
-            BitmapIconSource<TIconSource>::type icon;
+            typename BitmapIconSource<TIconSource>::type icon;
             icon.UriSource(nullptr);
             iconSource = icon;
         }
@@ -154,16 +164,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         return iconSource;
     }
 
-    static winrt::hstring _expandIconPath(hstring iconPath)
-    {
-        if (iconPath.empty())
-        {
-            return iconPath;
-        }
-        winrt::hstring envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(iconPath.c_str()) };
-        return envExpandedPath;
-    }
-
     // Method Description:
     // - Attempt to convert something into another type. For the
     //   IconPathConverter, we support a variety of icons:

src/cascadia/TerminalSettingsModel/JsonUtils.h

@@ -14,6 +14,8 @@ Author(s):
 
 #pragma once
 
+#define JSON_UTILS_H
+
 #include <json.h>
 
 #include "../types/inc/utils.hpp"
@@ -117,137 +119,9 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
         std::string expectedType;
     };
 
-    // Method Description:
-    // - Helper that will populate a reference with a value converted from a json object.
-    // Arguments:
-    // - json: the json object to convert
-    // - target: the value to populate with the converted result
-    // Return Value:
-    // - a boolean indicating whether the value existed (in this case, was non-null)
-    //
-    // GetValue, type-deduced, manual converter
-    template<typename T, typename Converter>
-    bool GetValue(const Json::Value& json, T& target, Converter&& conv)
-    {
-        if (!conv.CanConvert(json))
-        {
-            DeserializationError e{ json };
-            e.expectedType = conv.TypeDescription();
-            throw e;
-        }
-
-        target = conv.FromJson(json);
-        return true;
-    }
-
-    // GetValue, forced return type, manual converter
-    template<typename T, typename Converter>
-    std::decay_t<T> GetValue(const Json::Value& json, Converter&& conv)
-    {
-        std::decay_t<T> local{};
-        GetValue(json, local, std::forward<Converter>(conv));
-        return local; // returns zero-initialized or value
-    }
-
-    // GetValueForKey, type-deduced, manual converter
-    template<typename T, typename Converter>
-    bool GetValueForKey(const Json::Value& json, std::string_view key, T& target, Converter&& conv)
-    {
-        if (auto found{ json.find(&*key.cbegin(), (&*key.cbegin()) + key.size()) })
-        {
-            try
-            {
-                return GetValue(*found, target, std::forward<Converter>(conv));
-            }
-            catch (DeserializationError& e)
-            {
-                e.SetKey(key);
-                throw; // rethrow now that it has a key
-            }
-        }
-        return false;
-    }
-
-    // GetValueForKey, forced return type, manual converter
-    template<typename T, typename Converter>
-    std::decay_t<T> GetValueForKey(const Json::Value& json, std::string_view key, Converter&& conv)
-    {
-        std::decay_t<T> local{};
-        GetValueForKey(json, key, local, std::forward<Converter>(conv));
-        return local; // returns zero-initialized?
-    }
-
-    // GetValue, type-deduced, with automatic converter
-    template<typename T>
-    bool GetValue(const Json::Value& json, T& target)
-    {
-        return GetValue(json, target, ConversionTrait<typename std::decay<T>::type>{});
-    }
-
-    // GetValue, forced return type, with automatic converter
-    template<typename T>
-    std::decay_t<T> GetValue(const Json::Value& json)
-    {
-        std::decay_t<T> local{};
-        GetValue(json, local, ConversionTrait<typename std::decay<T>::type>{});
-        return local; // returns zero-initialized or value
-    }
-
-    // GetValueForKey, type-deduced, with automatic converter
-    template<typename T>
-    bool GetValueForKey(const Json::Value& json, std::string_view key, T& target)
-    {
-        return GetValueForKey(json, key, target, ConversionTrait<typename std::decay<T>::type>{});
-    }
-
-    // GetValueForKey, forced return type, with automatic converter
-    template<typename T>
-    std::decay_t<T> GetValueForKey(const Json::Value& json, std::string_view key)
-    {
-        return GetValueForKey<T>(json, key, ConversionTrait<typename std::decay<T>::type>{});
-    }
-
-    // Get multiple values for keys (json, k, &v, k, &v, k, &v, ...).
-    // Uses the default converter for each v.
-    // Careful: this can cause a template explosion.
-    constexpr void GetValuesForKeys(const Json::Value& /*json*/) {}
-
-    template<typename T, typename... Args>
-    void GetValuesForKeys(const Json::Value& json, std::string_view key1, T&& val1, Args&&... args)
-    {
-        GetValueForKey(json, key1, val1);
-        GetValuesForKeys(json, std::forward<Args>(args)...);
-    }
-
-    // SetValueForKey, type-deduced, manual converter
-    template<typename T, typename Converter>
-    void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
-    {
-        // We don't want to write any empty optionals into JSON (right now).
-        if (OptionOracle<T>::HasValue(target))
-        {
-            // demand guarantees that it will return a value or throw an exception
-            *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
-        }
-    }
-
-    // SetValueForKey, type-deduced, with automatic converter
-    template<typename T>
-    void SetValueForKey(Json::Value& json, std::string_view key, const T& target)
-    {
-        SetValueForKey(json, key, target, ConversionTrait<typename std::decay<T>::type>{});
-    }
-
     template<typename T>
     struct ConversionTrait
     {
-        // Forward-declare these so the linker can pick up specializations from elsewhere!
-        T FromJson(const Json::Value&);
-        bool CanConvert(const Json::Value& json);
-
-        Json::Value ToJson(const T& val);
-
-        std::string TypeDescription() const { return "<unknown>"; }
     };
 
     template<>
@@ -377,7 +251,8 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
 
         std::string TypeDescription() const
         {
-            return fmt::format("{}[]", ConversionTrait<GUID>{}.TypeDescription());
+            ConversionTrait<T> trait;
+            return fmt::format("{}[]", trait.TypeDescription());
         }
     };
 
@@ -421,7 +296,8 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
 
             for (const auto& [k, v] : val)
             {
-                SetValueForKey(json, k, v);
+                ConversionTrait<T> trait;
+                *json.demand(&*k.cbegin(), (&*k.cbegin()) + k.size()) = trait.ToJson(v);
             }
 
             return json;
@@ -435,7 +311,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
 
 #ifdef WINRT_BASE_H
     template<>
-    struct ConversionTrait<winrt::hstring> : public ConversionTrait<std::wstring>
+    struct ConversionTrait<winrt::hstring>
     {
         // Leverage the wstring converter's validation
         winrt::hstring FromJson(const Json::Value& json)
@@ -459,7 +335,12 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
         bool CanConvert(const Json::Value& json) const
         {
             // hstring has a specific behavior for null, so it can convert it
-            return ConversionTrait<std::wstring>::CanConvert(json) || json.isNull();
+            return ConversionTrait<std::wstring>{}.CanConvert(json) || json.isNull();
+        }
+
+        std::string TypeDescription() const
+        {
+            return "string";
         }
     };
 
@@ -539,7 +420,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
 
             for (const auto& [k, v] : val)
             {
-                SetValueForKey(json, til::u16u8(k), v);
+                //SetValueForKey(json, til::u16u8(k), v);
             }
 
             return json;
@@ -709,22 +590,26 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
     {
         winrt::guid FromJson(const Json::Value& json) const
         {
-            return static_cast<winrt::guid>(ConversionTrait<GUID>{}.FromJson(json));
+            ConversionTrait<GUID> trait;
+            return static_cast<winrt::guid>(trait.FromJson(json));
         }
 
         bool CanConvert(const Json::Value& json) const
         {
-            return ConversionTrait<GUID>{}.CanConvert(json);
+            ConversionTrait<GUID> trait;
+            return trait.CanConvert(json);
         }
 
         Json::Value ToJson(const winrt::guid& val)
         {
-            return ConversionTrait<GUID>{}.ToJson(val);
+            ConversionTrait<GUID> trait;
+            return trait.ToJson(val);
         }
 
         std::string TypeDescription() const
         {
-            return ConversionTrait<GUID>{}.TypeDescription();
+            ConversionTrait<GUID> trait;
+            return trait.TypeDescription();
         }
     };
 
@@ -762,11 +647,11 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
     template<>
     struct ConversionTrait<winrt::Windows::Foundation::Size>
     {
-        winrt::Windows::Foundation::Size FromJson(const Json::Value& json)
+        winrt::Windows::Foundation::Size FromJson(const Json::Value&)
         {
             winrt::Windows::Foundation::Size size{};
-            GetValueForKey(json, std::string_view("width"), size.Width);
-            GetValueForKey(json, std::string_view("height"), size.Height);
+            //GetValueForKey(json, std::string_view("width"), size.Width);
+            //GetValueForKey(json, std::string_view("height"), size.Height);
 
             return size;
         }
@@ -781,12 +666,12 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
             return json.isMember("width") && json.isMember("height");
         }
 
-        Json::Value ToJson(const winrt::Windows::Foundation::Size& val)
+        Json::Value ToJson(const winrt::Windows::Foundation::Size&)
         {
             Json::Value json{ Json::objectValue };
 
-            SetValueForKey(json, std::string_view("width"), val.Width);
-            SetValueForKey(json, std::string_view("height"), val.Height);
+            //SetValueForKey(json, std::string_view("width"), val.Width);
+            //SetValueForKey(json, std::string_view("height"), val.Height);
 
             return json;
         }
@@ -804,22 +689,26 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
     {
         winrt::Windows::UI::Color FromJson(const Json::Value& json) const
         {
-            return static_cast<winrt::Windows::UI::Color>(ConversionTrait<til::color>{}.FromJson(json));
+            ConversionTrait<til::color> trait;
+            return static_cast<winrt::Windows::UI::Color>(trait.FromJson(json));
         }
 
         bool CanConvert(const Json::Value& json) const
         {
-            return ConversionTrait<til::color>{}.CanConvert(json);
+            ConversionTrait<til::color> trait;
+            return trait.CanConvert(json);
         }
 
         Json::Value ToJson(const winrt::Windows::UI::Color& val)
         {
-            return ConversionTrait<til::color>{}.ToJson(val);
+            ConversionTrait<til::color> trait;
+            return trait.ToJson(val);
         }
 
         std::string TypeDescription() const
         {
-            return ConversionTrait<til::color>{}.TypeDescription();
+            ConversionTrait<til::color> trait;
+            return trait.TypeDescription();
         }
     };
 #endif
@@ -1055,6 +944,131 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
             return "any";
         }
     };
+
+    // Method Description:
+    // - Helper that will populate a reference with a value converted from a json object.
+    // Arguments:
+    // - json: the json object to convert
+    // - target: the value to populate with the converted result
+    // Return Value:
+    // - a boolean indicating whether the value existed (in this case, was non-null)
+    //
+    // GetValue, type-deduced, manual converter
+    template<typename T, typename Converter>
+    bool GetValue(const Json::Value& json, T& target, Converter&& conv)
+    {
+        if (!conv.CanConvert(json))
+        {
+            DeserializationError e{ json };
+            e.expectedType = conv.TypeDescription();
+            throw e;
+        }
+
+        target = conv.FromJson(json);
+        return true;
+    }
+
+    // GetValue, forced return type, manual converter
+    template<typename T, typename Converter>
+    std::decay_t<T> GetValue(const Json::Value& json, Converter&& conv)
+    {
+        std::decay_t<T> local{};
+        GetValue(json, local, std::forward<Converter>(conv));
+        return local; // returns zero-initialized or value
+    }
+
+    // GetValueForKey, type-deduced, manual converter
+    template<typename T, typename Converter>
+    bool GetValueForKey(const Json::Value& json, std::string_view key, T& target, Converter&& conv)
+    {
+        if (auto found{ json.find(&*key.cbegin(), (&*key.cbegin()) + key.size()) })
+        {
+            try
+            {
+                return GetValue(*found, target, std::forward<Converter>(conv));
+            }
+            catch (DeserializationError& e)
+            {
+                e.SetKey(key);
+                throw; // rethrow now that it has a key
+            }
+        }
+        return false;
+    }
+
+    // GetValueForKey, forced return type, manual converter
+    template<typename T, typename Converter>
+    std::decay_t<T> GetValueForKey(const Json::Value& json, std::string_view key, Converter&& conv)
+    {
+        std::decay_t<T> local{};
+        GetValueForKey(json, key, local, std::forward<Converter>(conv));
+        return local; // returns zero-initialized?
+    }
+
+    // GetValue, type-deduced, with automatic converter
+    template<typename T>
+    bool GetValue(const Json::Value& json, T& target)
+    {
+        ConversionTrait<std::decay_t<T>> trait;
+        return GetValue(json, target, trait);
+    }
+
+    // GetValue, forced return type, with automatic converter
+    template<typename T>
+    std::decay_t<T> GetValue(const Json::Value& json)
+    {
+        std::decay_t<T> local{};
+        ConversionTrait<std::decay_t<T>> trait;
+        GetValue(json, local, trait);
+        return local; // returns zero-initialized or value
+    }
+
+    // GetValueForKey, type-deduced, with automatic converter
+    template<typename T>
+    bool GetValueForKey(const Json::Value& json, std::string_view key, T& target)
+    {
+        ConversionTrait<std::decay_t<T>> trait;
+        return GetValueForKey(json, key, target, trait);
+    }
+
+    // GetValueForKey, forced return type, with automatic converter
+    template<typename T>
+    std::decay_t<T> GetValueForKey(const Json::Value& json, std::string_view key)
+    {
+        ConversionTrait<std::decay_t<T>> trait;
+        return GetValueForKey<T>(json, key, trait);
+    }
+
+    // Get multiple values for keys (json, k, &v, k, &v, k, &v, ...).
+    // Uses the default converter for each v.
+    // Careful: this can cause a template explosion.
+    constexpr void GetValuesForKeys(const Json::Value& /*json*/) {}
+
+    template<typename T, typename... Args>
+    void GetValuesForKeys(const Json::Value& json, std::string_view key1, T&& val1, Args&&... args)
+    {
+        GetValueForKey(json, key1, val1);
+        GetValuesForKeys(json, std::forward<Args>(args)...);
+    }
+
+    // SetValueForKey, type-deduced, manual converter
+    template<typename T, typename Converter>
+    void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
+    {
+        // We don't want to write any empty optionals into JSON (right now).
+        if (OptionOracle<T>::HasValue(target))
+        {
+            // demand guarantees that it will return a value or throw an exception
+            *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
+        }
+    }
+
+    // SetValueForKey, type-deduced, with automatic converter
+    template<typename T>
+    void SetValueForKey(Json::Value& json, std::string_view key, const T& target)
+    {
+        SetValueForKey(json, key, target, ConversionTrait<typename std::decay<T>::type>{});
+    }
 };
 
 #define JSON_ENUM_MAPPER(...)                                                                \

src/cascadia/TerminalSettingsModel/KeyChordSerialization.h

@@ -4,7 +4,6 @@
 #pragma once
 
 #include "KeyChordSerialization.g.h"
-#include "JsonUtils.h"
 #include "../inc/cppwinrt_utils.h"
 
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation

src/cascadia/TerminalSettingsModel/Profile.h

@@ -49,7 +49,6 @@ Author(s):
 #include "MTSMSettings.h"
 
 #include "../inc/cppwinrt_utils.h"
-#include "JsonUtils.h"
 #include <DefaultSettings.h>
 #include "AppearanceConfig.h"
 #include "FontConfig.h"

src/cascadia/TerminalSettingsModel/pch.h

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 //
 // pch.h
@@ -43,11 +43,13 @@
 
 // Including TraceLogging essentials for the binary
 #include <TraceLoggingProvider.h>
-#include <winmeta.h>
 TRACELOGGING_DECLARE_PROVIDER(g_hSettingsModelProvider);
 #include <telemetry/ProjectTelemetry.h>
 #include <TraceLoggingActivity.h>
 
+#include <winmeta.h>
+#include <ShlObj_core.h>
+
 // JsonCpp
 #include <json.h>
 

src/common.build.pre.props

@@ -116,9 +116,11 @@
       <MultiProcessorCompilation>true</MultiProcessorCompilation>
       <MinimalRebuild>false</MinimalRebuild>
       <RuntimeTypeInfo>false</RuntimeTypeInfo>
+      <ConformanceMode>true</ConformanceMode>
+      <UseStandardPreprocessor>true</UseStandardPreprocessor>
       <LanguageStandard>stdcpp17</LanguageStandard>
       <LanguageStandard_C>stdc17</LanguageStandard_C>
-      <AdditionalOptions>/utf-8 %(AdditionalOptions)</AdditionalOptions>
+      <AdditionalOptions>%(AdditionalOptions) /utf-8 /Zc:externConstexpr /Zc:lambda /Zc:throwingNew</AdditionalOptions>
       <ControlFlowGuard>Guard</ControlFlowGuard>
       <FloatingPointModel>Fast</FloatingPointModel>
     </ClCompile>

src/cppwinrt.build.pre.props

@@ -72,8 +72,6 @@
       <PrecompiledHeaderOutputFile>$(IntDir)pch.pch</PrecompiledHeaderOutputFile>
 
       <!-- All new code should be in non-permissive mode. Big objects for C++/WinRT. -->
-      <ConformanceMode>true</ConformanceMode>
-      <UseStandardPreprocessor>true</UseStandardPreprocessor>
       <AdditionalOptions>%(AdditionalOptions) /bigobj /Zc:twoPhase-</AdditionalOptions>
       <DisableSpecificWarnings>5104;28204;%(DisableSpecificWarnings)</DisableSpecificWarnings>
 

src/host/init.cpp

@@ -12,8 +12,6 @@
 // - Ensures the SxS initialization for the process.
 void InitSideBySide(_Out_writes_(ScratchBufferSize) PWSTR ScratchBuffer, __range(MAX_PATH, MAX_PATH) DWORD ScratchBufferSize)
 {
-    ACTCTXW actctx = { 0 };
-
     // Account for the fact that sidebyside stuff happens in CreateProcess
     // but conhost is run with RtlCreateUserProcess.
 
@@ -30,38 +28,35 @@ void InitSideBySide(_Out_writes_(ScratchBufferSize) PWSTR ScratchBuffer, __range
     // make references to DLLs in the system that are in the SxS cache (ex. a 3rd party IME is loaded and asks for
     // comctl32.dll. The load will fail if SxS wasn't initialized.) This was bug# WIN7:681280.
 
-    // We look at the first few chars without being careful about a terminal nul, so init them.
-    ScratchBuffer[0] = 0;
-    ScratchBuffer[1] = 0;
-    ScratchBuffer[2] = 0;
-    ScratchBuffer[3] = 0;
-    ScratchBuffer[4] = 0;
-    ScratchBuffer[5] = 0;
-    ScratchBuffer[6] = 0;
-
-    // GetModuleFileNameW truncates its result to fit in the buffer, so to detect if we fit, we have to do this.
-    ScratchBuffer[ScratchBufferSize - 2] = 0;
     DWORD const dwModuleFileNameLength = GetModuleFileNameW(nullptr, ScratchBuffer, ScratchBufferSize);
     if (dwModuleFileNameLength == 0)
     {
         RIPMSG1(RIP_ERROR, "GetModuleFileNameW failed %d.\n", GetLastError());
-        goto Exit;
+        return;
     }
-    if (ScratchBuffer[ScratchBufferSize - 2] != 0)
+    // GetModuleFileNameW truncates its result to fit in the buffer
+    // and returns the given buffer size in such cases.
+    if (dwModuleFileNameLength == ScratchBufferSize)
     {
         RIPMSG1(RIP_ERROR, "GetModuleFileNameW requires more than ScratchBufferSize(%d) - 1.\n", ScratchBufferSize);
-        goto Exit;
+        return;
     }
 
     // We get an NT path from the Win32 api. Fix it to be Win32.
+    // We can test for NT paths by checking whether the string starts with "\??\C:\", or any
+    // alternative letter other than C. We specifically don't test for the drive letter below.
     UINT NtToWin32PathOffset = 0;
-    if (ScratchBuffer[0] == '\\' && ScratchBuffer[1] == '?' && ScratchBuffer[2] == '?' && ScratchBuffer[3] == '\\'
-        //&& ScratchBuffer[4] == a drive letter
-        && ScratchBuffer[5] == ':' && ScratchBuffer[6] == '\\')
+    static constexpr wchar_t ntPathSpec1[]{ L'\\', L'?', L'?', L'\\' };
+    static constexpr wchar_t ntPathSpec2[]{ L':', L'\\' };
+    if (
+        dwModuleFileNameLength >= 7 &&
+        memcmp(&ScratchBuffer[0], &ntPathSpec1[0], sizeof(ntPathSpec1)) == 0 &&
+        memcmp(&ScratchBuffer[5], &ntPathSpec2[0], sizeof(ntPathSpec2)) == 0)
     {
         NtToWin32PathOffset = 4;
     }
 
+    ACTCTXW actctx{};
     actctx.cbSize = sizeof(actctx);
     actctx.dwFlags = (ACTCTX_FLAG_RESOURCE_NAME_VALID | ACTCTX_FLAG_SET_PROCESS_DEFAULT);
     actctx.lpResourceName = MAKEINTRESOURCE(IDR_SYSTEM_MANIFEST);
@@ -83,11 +78,7 @@ void InitSideBySide(_Out_writes_(ScratchBufferSize) PWSTR ScratchBuffer, __range
         {
             RIPMSG1(RIP_WARNING, "InitSideBySide failed create an activation context. Error: %d\r\n", error);
         }
-        goto Exit;
     }
-
-Exit:
-    ScratchBuffer[0] = 0;
 }
 
 // Routine Description:

src/host/tracing.cpp

@@ -58,7 +58,7 @@ Tracing::~Tracing()
 // Return Value:
 // - An object for the caller to hold until the API call is complete.
 //   Then destroy it to signal that the call is over so the stop trace can be written.
-Tracing Tracing::s_TraceApiCall(const NTSTATUS& result, PCSTR traceName)
+Tracing Tracing::s_TraceApiCall(const NTSTATUS result, PCSTR traceName)
 {
     // clang-format off
     TraceLoggingWrite(

src/host/tracing.hpp

@@ -40,7 +40,7 @@ class Tracing
 public:
     ~Tracing();
 
-    static Tracing s_TraceApiCall(const NTSTATUS& result, PCSTR traceName);
+    static Tracing s_TraceApiCall(const NTSTATUS result, PCSTR traceName);
 
     static void s_TraceApi(const NTSTATUS status, const CONSOLE_GETLARGESTWINDOWSIZE_MSG* const a);
     static void s_TraceApi(const NTSTATUS status, const CONSOLE_SCREENBUFFERINFO_MSG* const a, const bool fSet);

src/inc/til/static_map.h

@@ -97,7 +97,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
     public:
         template<typename... Args>
         constexpr explicit presorted_static_map(const Args&... args) noexcept :
-            static_map{ args... } {};
+            static_map<K, V, Compare, N, details::presorted_input_t>{ args... } {};
     };
 
     // this is a deduction guide that ensures two things:

src/inc/til/string.h

@@ -169,7 +169,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
     // and 5x or more for long strings (128 characters or more).
     // See: https://github.com/microsoft/STL/issues/2289
     template<typename T, typename Traits>
-    bool equals(const std::basic_string_view<T, Traits>& str1, const std::basic_string_view<T, Traits>& str2) noexcept
+    bool equals(const std::basic_string_view<T, Traits>& lhs, const std::basic_string_view<T, Traits>& rhs) noexcept
     {
         return lhs.size() == rhs.size() && __builtin_memcmp(lhs.data(), rhs.data(), lhs.size() * sizeof(T)) == 0;
     }

src/propsheet/PropSheetHandler.cpp

@@ -18,14 +18,7 @@ static CONSOLE_STATE_INFO g_csi;
 using namespace Microsoft::WRL;
 
 // This class exposes console property sheets for use when launching the filesystem shortcut properties dialog.
-// clang-format off
-[uuid(D2942F8E-478E-41D3-870A-35A16238F4EE)]
-class ConsolePropertySheetHandler WrlFinal : public RuntimeClass<RuntimeClassFlags<ClassicCom>,
-                                                                 IShellExtInit,
-                                                                 IShellPropSheetExt,
-                                                                 IPersist,
-                                                                 FtmBase>
-// clang-format on
+class __declspec(uuid("D2942F8E-478E-41D3-870A-35A16238F4EE")) ConsolePropertySheetHandler final : public RuntimeClass<RuntimeClassFlags<ClassicCom>, IShellExtInit, IShellPropSheetExt, IPersist, FtmBase>
 {
 public:
     HRESULT RuntimeClassInitialize()
@@ -34,7 +27,7 @@ public:
     }
 
     // IPersist
-    STDMETHODIMP GetClassID(_Out_ CLSID * clsid) override
+    STDMETHODIMP GetClassID(_Out_ CLSID* clsid) override
     {
         *clsid = __uuidof(this);
         return S_OK;
@@ -43,7 +36,7 @@ public:
     // IShellExtInit
     // Shell QI's for IShellExtInit and calls Initialize first. If we return a succeeding HRESULT, the shell will QI for
     // IShellPropSheetExt and call AddPages. A failing HRESULT causes the shell to skip us.
-    STDMETHODIMP Initialize(_In_ PCIDLIST_ABSOLUTE /*pidlFolder*/, _In_ IDataObject * pdtobj, _In_ HKEY /*hkeyProgID*/)
+    STDMETHODIMP Initialize(_In_ PCIDLIST_ABSOLUTE /*pidlFolder*/, _In_ IDataObject* pdtobj, _In_ HKEY /*hkeyProgID*/)
     {
         WCHAR szLinkFileName[MAX_PATH];
         HRESULT hr = _ShouldAddPropertySheet(pdtobj, szLinkFileName, ARRAYSIZE(szLinkFileName));
@@ -139,7 +132,7 @@ private:
     ///////////////////////////////////////////////////////////////////////////
     // CODE FROM THE SHELL DEPOT'S `idllib.h`
     // get a link target item without resolving it.
-    HRESULT GetTargetIdList(_In_ IShellItem * psiLink, _COM_Outptr_ PIDLIST_ABSOLUTE * ppidl)
+    HRESULT GetTargetIdList(_In_ IShellItem* psiLink, _COM_Outptr_ PIDLIST_ABSOLUTE* ppidl)
     {
         *ppidl = nullptr;
 
@@ -156,7 +149,7 @@ private:
         }
         return hr;
     }
-    HRESULT GetTargetItem(_In_ IShellItem * psiLink, _In_ REFIID riid, _COM_Outptr_ void** ppv)
+    HRESULT GetTargetItem(_In_ IShellItem* psiLink, _In_ REFIID riid, _COM_Outptr_ void** ppv)
     {
         *ppv = nullptr;
 
@@ -171,7 +164,7 @@ private:
     }
     ///////////////////////////////////////////////////////////////////////////
 
-    HRESULT _GetShellItemLinkTargetExpanded(_In_ IShellItem * pShellItem,
+    HRESULT _GetShellItemLinkTargetExpanded(_In_ IShellItem* pShellItem,
                                             _Out_writes_(cchFilePathExtended) PWSTR pszFilePathExtended,
                                             const size_t cchFilePathExtended)
     {
@@ -190,7 +183,7 @@ private:
         return hr;
     }
 
-    HRESULT _ShouldAddPropertySheet(_In_ IDataObject * pdtobj,
+    HRESULT _ShouldAddPropertySheet(_In_ IDataObject* pdtobj,
                                     _Out_writes_(cchLinkFileName) PWSTR pszLinkFileName,
                                     const size_t cchLinkFileName)
     {

src/renderer/atlas/AtlasEngine.h

@@ -359,7 +359,9 @@ namespace Microsoft::Console::Render
 
             bool is_inline() const noexcept
             {
-                return (__builtin_bit_cast(uintptr_t, allocated) & 1) != 0;
+                // VSO-1430353: __builtin_bitcast crashes the compiler under /permissive-.
+#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1).
+                return (reinterpret_cast<uintptr_t>(allocated) & 1) != 0;
             }
 
             const T* data() const noexcept

src/server/ApiSorter.cpp

@@ -144,11 +144,10 @@ PCONSOLE_API_MSG ApiSorter::ConsoleDispatchRequest(_Inout_ PCONSOLE_API_MSG Mess
     ULONG const LayerNumber = (Message->msgHeader.ApiNumber >> 24) - 1;
     ULONG const ApiNumber = Message->msgHeader.ApiNumber & 0xffffff;
 
-    NTSTATUS Status;
-    if ((LayerNumber >= RTL_NUMBER_OF(ConsoleApiLayerTable)) || (ApiNumber >= ConsoleApiLayerTable[LayerNumber].Count))
+    if ((LayerNumber >= std::size(ConsoleApiLayerTable)) || (ApiNumber >= ConsoleApiLayerTable[LayerNumber].Count))
     {
-        Status = STATUS_ILLEGAL_FUNCTION;
-        goto Complete;
+        Message->SetReplyStatus(STATUS_ILLEGAL_FUNCTION);
+        return Message;
     }
 
     CONSOLE_API_DESCRIPTOR const* Descriptor = &ConsoleApiLayerTable[LayerNumber].Descriptor[ApiNumber];
@@ -159,8 +158,8 @@ PCONSOLE_API_MSG ApiSorter::ConsoleDispatchRequest(_Inout_ PCONSOLE_API_MSG Mess
         (Message->msgHeader.ApiDescriptorSize > Message->Descriptor.InputSize - sizeof(CONSOLE_MSG_HEADER)) ||
         (Message->msgHeader.ApiDescriptorSize < Descriptor->RequiredSize))
     {
-        Status = STATUS_ILLEGAL_FUNCTION;
-        goto Complete;
+        Message->SetReplyStatus(STATUS_ILLEGAL_FUNCTION);
+        return Message;
     }
 
     BOOL ReplyPending = FALSE;
@@ -173,6 +172,7 @@ PCONSOLE_API_MSG ApiSorter::ConsoleDispatchRequest(_Inout_ PCONSOLE_API_MSG Mess
     // hard dependencies on NTSTATUS codes that aren't readily expressible as an HRESULT. There's currently only one
     // such known code -- STATUS_BUFFER_TOO_SMALL. There's a conlibk dependency on this being returned from the console
     // alias API.
+    NTSTATUS Status = S_OK;
     {
         const auto trace = Tracing::s_TraceApiCall(Status, Descriptor->TraceName);
         Status = (*Descriptor->Routine)(Message, &ReplyPending);
@@ -184,14 +184,9 @@ PCONSOLE_API_MSG ApiSorter::ConsoleDispatchRequest(_Inout_ PCONSOLE_API_MSG Mess
 
     if (!ReplyPending)
     {
-        goto Complete;
+        Message->SetReplyStatus(Status);
+        return Message;
     }
 
     return nullptr;
-
-Complete:
-
-    Message->SetReplyStatus(Status);
-
-    return Message;
 }

src/server/IoDispatchers.cpp

@@ -92,7 +92,9 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleCreateObject(_In_ PCONSOLE_API_MSG pMessa
 
     if (!NT_SUCCESS(Status))
     {
-        goto Error;
+        UnlockConsole();
+        pMessage->SetReplyStatus(Status);
+        return pMessage;
     }
 
     auto deviceComm{ ServiceLocator::LocateGlobals().pDeviceComm };
@@ -110,16 +112,6 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleCreateObject(_In_ PCONSOLE_API_MSG pMessa
     UnlockConsole();
 
     return nullptr;
-
-Error:
-
-    FAIL_FAST_IF(NT_SUCCESS(Status));
-
-    UnlockConsole();
-
-    pMessage->SetReplyStatus(Status);
-
-    return pMessage;
 }
 
 // Routine Description:
@@ -256,17 +248,32 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
     Telemetry::Instance().LogApiCall(Telemetry::ApiCall::AttachConsole);
 
     ConsoleProcessHandle* ProcessData = nullptr;
+    NTSTATUS Status;
 
     LockConsole();
 
+    const auto cleanup = wil::scope_exit([&]() noexcept {
+        UnlockConsole();
+
+        if (!NT_SUCCESS(Status))
+        {
+            pReceiveMsg->SetReplyStatus(Status);
+            if (ProcessData != nullptr)
+            {
+                CommandHistory::s_Free(ProcessData);
+                gci.ProcessHandleList.FreeProcessData(ProcessData);
+            }
+        }
+    });
+
     DWORD const dwProcessId = (DWORD)pReceiveMsg->Descriptor.Process;
     DWORD const dwThreadId = (DWORD)pReceiveMsg->Descriptor.Object;
 
     CONSOLE_API_CONNECTINFO Cac;
-    NTSTATUS Status = ConsoleInitializeConnectInfo(pReceiveMsg, &Cac);
+    Status = ConsoleInitializeConnectInfo(pReceiveMsg, &Cac);
     if (!NT_SUCCESS(Status))
     {
-        goto Error;
+        return pReceiveMsg;
     }
 
     // If we pass the tests...
@@ -354,7 +361,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
 
     if (!NT_SUCCESS(Status))
     {
-        goto Error;
+        return pReceiveMsg;
     }
 
     ProcessData->fRootProcess = WI_IsFlagClear(gci.Flags, CONSOLE_INITIALIZED);
@@ -376,7 +383,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
         Status = ConsoleAllocateConsole(&Cac);
         if (!NT_SUCCESS(Status))
         {
-            goto Error;
+            return pReceiveMsg;
         }
 
         WI_SetFlag(gci.Flags, CONSOLE_INITIALIZED);
@@ -389,7 +396,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
     catch (...)
     {
         LOG_CAUGHT_EXCEPTION();
-        goto Error;
+        return pReceiveMsg;
     }
 
     gci.ProcessHandleList.ModifyConsoleProcessFocus(WI_IsFlagSet(gci.Flags, CONSOLE_HAS_FOCUS));
@@ -403,7 +410,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
 
     if (!NT_SUCCESS(Status))
     {
-        goto Error;
+        return pReceiveMsg;
     }
 
     auto& screenInfo = gci.GetActiveOutputBuffer().GetMainBuffer();
@@ -414,7 +421,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
 
     if (!NT_SUCCESS(Status))
     {
-        goto Error;
+        return pReceiveMsg;
     }
 
     // Complete the request.
@@ -434,24 +441,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
 
     Tracing::s_TraceConsoleAttachDetach(ProcessData, true);
 
-    UnlockConsole();
-
     return nullptr;
-
-Error:
-    FAIL_FAST_IF(NT_SUCCESS(Status));
-
-    if (ProcessData != nullptr)
-    {
-        CommandHistory::s_Free((HANDLE)ProcessData);
-        gci.ProcessHandleList.FreeProcessData(ProcessData);
-    }
-
-    UnlockConsole();
-
-    pReceiveMsg->SetReplyStatus(Status);
-
-    return pReceiveMsg;
 }
 
 // Routine Description: