CLANG: Ordering matters

This reverts commit 6e6d3dc59ed5c677e1a1ee14d19ae1ca653672ed.

build/pipelines/ci.yml

@@ -107,3 +107,35 @@ stages:
               platform: ${{ platform }}
               # The tests might be run more than once; log one artifact per attempt.
               outputArtifactStem: -$(System.JobAttempt)
+
+  - stage: Build_x64_Clang
+    displayName: Build x64 ClangCL
+    dependsOn: []
+    jobs:
+      - template: ./templates-v2/job-build-project.yml
+        parameters:
+          pool:
+            ${{ if eq(variables['System.CollectionId'], 'cb55739e-4afe-46a3-970f-1b49d8ee7564') }}:
+              name: SHINE-INT-L
+            ${{ else }}:
+              name: SHINE-OSS-L
+          buildPlatforms:
+            - x64
+          buildConfigurations: [Release]
+          buildEverything: true
+          keepAllExpensiveBuildOutputs: false
+          additionalBuildOptions: /p:WindowsTerminalClangBuild=true
+          artifactStem: -clang
+
+  - stage: Test_x64_Clang
+    displayName: Test x64 ClangCL
+    dependsOn:
+      - Build_x64_Clang
+    condition: succeeded()
+    jobs:
+      - template: ./templates-v2/job-test-project.yml
+        parameters:
+          platform: x64
+          # The tests might be run more than once; log one artifact per attempt.
+          inputArtifactStem: -clang
+          outputArtifactStem: -clang-$(System.JobAttempt)

scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj

@@ -8,8 +8,7 @@
     <ProjectName>WindowExe</ProjectName>
     <TargetName>WindowExe</TargetName>
     <ConfigurationType>Application</ConfigurationType>
-    <OpenConsoleUniversalApp>false</OpenConsoleUniversalApp>
-    <ApplicationType>Windows Store</ApplicationType>
+    <OpenConsoleUniversalApp>true</OpenConsoleUniversalApp>
     <WindowsStoreApp>true</WindowsStoreApp>
     <WindowsAppContainer>false</WindowsAppContainer>
     <TargetPlatformIdentifier>Windows</TargetPlatformIdentifier>

src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.vcxproj

@@ -31,6 +31,13 @@
   <Import Project="$(OpenConsoleDir)src\common.nugetversions.props" />
   <Import Project="$(OpenConsoleDir)\src\cppwinrt.build.pre.props" />
 
+  <!-- Clang runs out of memory building the PCH, so disable it for this project -->
+  <ItemDefinitionGroup Condition="'$(PlatformToolset)'=='ClangCL'">
+    <ClCompile>
+      <PrecompiledHeader>NotUsing</PrecompiledHeader>
+    </ClCompile>
+  </ItemDefinitionGroup>
+
   <!-- ========================= Headers ======================== -->
   <ItemGroup>
     <ClInclude Include="pch.h" />
@@ -43,7 +50,7 @@
     <ClCompile Include="SettingsTests.cpp" />
     <ClCompile Include="TabTests.cpp" />
 	<ClCompile Include="FilteredCommandTests.cpp" />
-    <ClCompile Include="pch.cpp">
+    <ClCompile Include="pch.cpp" Condition="'$(PlatformToolset)'!='ClangCL'">
       <PrecompiledHeader>Create</PrecompiledHeader>
     </ClCompile>
   </ItemGroup>

src/cascadia/LocalTests_TerminalApp/TestHostApp/TestHostApp.vcxproj

@@ -32,6 +32,8 @@
      -->
     <AppxOSMinVersionReplaceManifestVersion>false</AppxOSMinVersionReplaceManifestVersion>
     <AppxOSMaxVersionTestedReplaceManifestVersion>false</AppxOSMaxVersionTestedReplaceManifestVersion>
+
+    <OpenConsoleProjectRequiresMSVC>true</OpenConsoleProjectRequiresMSVC>
   </PropertyGroup>
 
   <Import Project="$(SolutionDir)\common.openconsole.props" Condition="'$(OpenConsoleDir)'==''" />

src/cascadia/TerminalControl/ScrollBarVisualStateManager.h

@@ -14,6 +14,8 @@
 #include <winrt/Windows.UI.Xaml.h>
 #include <winrt/Windows.UI.Xaml.Controls.h>
 
+#include "TermControl.h"
+
 #include "ScrollBarVisualStateManager.g.h"
 
 namespace winrt::Microsoft::Terminal::Control::implementation

src/cascadia/TerminalControl/TermControlAutomationPeer.h

@@ -33,6 +33,7 @@ Modifications:
 #include "../types/TermControlUiaProvider.hpp"
 #include "../types/IUiaEventDispatcher.h"
 #include "../types/IControlAccessibilityInfo.h"
+#include "TermControl.h"
 
 namespace winrt::Microsoft::Terminal::Control::implementation
 {

src/cascadia/WindowsTerminal/BaseWindow.h

@@ -9,6 +9,8 @@ class BaseWindow
 public:
     static constexpr UINT CM_UPDATE_TITLE = WM_USER + 0;
 
+    virtual ~BaseWindow() = default;
+
     static T* GetThisFromHandle(HWND const window) noexcept
     {
         return reinterpret_cast<T*>(GetWindowLongPtr(window, GWLP_USERDATA));

src/cascadia/WindowsTerminal/WindowsTerminal.vcxproj

@@ -8,10 +8,9 @@
     <ProjectName>WindowsTerminal</ProjectName>
     <TargetName>WindowsTerminal</TargetName>
     <ConfigurationType>Application</ConfigurationType>
-    <OpenConsoleUniversalApp>false</OpenConsoleUniversalApp>
-    <ApplicationType>Windows Store</ApplicationType>
-    <WindowsStoreApp>true</WindowsStoreApp>
-    <WindowsAppContainer>false</WindowsAppContainer>
+    <OpenConsoleUniversalApp>true</OpenConsoleUniversalApp>
+    <AppContainerApplication>false</AppContainerApplication>
+    <!-- If we do not set this, it will be derived as UAP by our common build rules; this will really blow up the packaging project -->
     <TargetPlatformIdentifier>Windows</TargetPlatformIdentifier>
     <VersionInfoFileDescription>Windows Terminal Host</VersionInfoFileDescription>
     <PgoTarget>true</PgoTarget>
@@ -105,6 +104,14 @@
       <Private>false</Private>
       <CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies>
     </Reference>
+    <Reference Include="$(WindowsSDK_MetadataPathVersioned)\Windows.UI.Xaml.Hosting.HostingContract\*\*.winmd">
+      <WinMDFile>true</WinMDFile>
+      <CopyLocal>false</CopyLocal>
+      <ReferenceGrouping>$(TargetPlatformMoniker)</ReferenceGrouping>
+      <ReferenceGroupingDisplayName>$(TargetPlatformDisplayName)</ReferenceGroupingDisplayName>
+      <ResolvedFrom>CppWinRTImplicitlyExpandTargetPlatform</ResolvedFrom>
+      <IsSystemReference>True</IsSystemReference>
+    </Reference>
   </ItemGroup>
   <!--
     This ItemGroup and the Globals PropertyGroup below it are required in order

src/cascadia/ut_app/JsonUtilsTests.cpp

@@ -332,7 +332,7 @@ namespace TerminalAppUnitTests
         // test FromJson
         Json::Value jsonObject{ json };
         const auto value{ GetValue<TExpected>(jsonObject) };
-        VERIFY_ARE_EQUAL(expected, value, NoThrowString{}.Format(L"(type: %hs)", typeid(TExpected).name()));
+        VERIFY_ARE_EQUAL(expected, value); // NoThrowString{}.Format(L"(type: %hs)", typeid(TExpected).name()));
 
         // test ToJson
         {

src/common.build.post.props

@@ -49,7 +49,7 @@
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
     </ClCompile>
     <Link>
-      <AdditionalDependencies>onecoreuap_apiset.lib;d3dcompiler.lib;dwmapi.lib;uxtheme.lib;shlwapi.lib;ntdll.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies>onecoreuap_apiset.lib;usp10.lib;d3dcompiler.lib;dwmapi.lib;uxtheme.lib;shlwapi.lib;ntdll.lib;%(AdditionalDependencies)</AdditionalDependencies>
     </Link>
   </ItemDefinitionGroup>
 
@@ -67,6 +67,15 @@
     </ClCompile>
   </ItemDefinitionGroup>
 
+  <!--
+    If we do not override the target platform, the AppX packaging rules (which we honestly don't even care to use...) will
+    fail because they're looking for an SDK called "Clang.Windows". Yeah.
+  -->
+  <PropertyGroup Condition="'$(OpenConsoleUniversalApp)'=='true' and '$(PlatformToolset)'=='ClangCL'">
+    <TargetPlatformIdentifier Condition="'$(_OpenConsoleProjectProvidedTargetPlatformIdentifier)'==''">$(_OpenConsoleProjectProvidedTargetPlatformIdentifier)</TargetPlatformIdentifier>
+    <TargetPlatformIdentifier Condition="'$(TargetPlatformIdentifier)'==''">UAP</TargetPlatformIdentifier>
+  </PropertyGroup>
+
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
 
   <!-- Exclude our dependencies from static analysis. CAExcludePath can only be
@@ -74,6 +83,10 @@
   <PropertyGroup>
     <CAExcludePath>$(SolutionDir)\dep\;$(CAExcludePath)</CAExcludePath>
   </PropertyGroup>
+  <PropertyGroup>
+	  <LinkToolPath></LinkToolPath>
+	  <LinkToolExe></LinkToolExe>
+  </PropertyGroup>
 
   <Target Name="_ComputePrecompToCleanUp">
     <ItemGroup>

src/common.build.pre.props

@@ -13,6 +13,8 @@
 
     <IntDir>$(SolutionDir)obj\$(Platform)\$(Configuration)\$(ProjectName)\</IntDir>
     <IntermediateOutputPath>$(IntDir)</IntermediateOutputPath>
+    <EnableClangTidyCodeAnalysis>true</EnableClangTidyCodeAnalysis>
+    <ClangTidyChecks>-*,modernize-*,performance-*,-modernize-use-trailing-return-type</ClangTidyChecks>
 
     <!-- Both pairs of OutDir/OutputPath and IntDir/IntermediateOutputPath must be set;
          different parts of the project infrastructure use them (without rhyme or reason.) -->
@@ -95,10 +97,10 @@
 
   <!-- For ALL build types-->
   <PropertyGroup Label="Configuration">
-    <PlatformToolset>v143</PlatformToolset>
+    <PlatformToolset Condition="'$(OpenConsoleProjectRequiresMSVC)'!='true' and '$(WindowsTerminalClangBuild)'=='true'">ClangCL</PlatformToolset>
+    <PlatformToolset Condition="'$(PlatformToolset)'==''">v143</PlatformToolset>
     <CharacterSet>Unicode</CharacterSet>
     <LinkIncremental>false</LinkIncremental>
-    <PreferredToolArchitecture>x64</PreferredToolArchitecture>
     <EnableHybridCRT Condition="'$(EnableHybridCRT)'=='' and '$(ConfigurationSupportsHybridCRT)'=='true'">true</EnableHybridCRT>
     <UseCrtSDKReferenceStaticWarning Condition="'$(EnableHybridCRT)'=='true'">false</UseCrtSDKReferenceStaticWarning>
     <!--
@@ -116,7 +118,7 @@
       <WarningLevel>Level4</WarningLevel>
       <TreatSpecificWarningsAsErrors>4189;4100;4242;4389;4244</TreatSpecificWarningsAsErrors>
       <!--<WarningLevel>EnableAllWarnings</WarningLevel>-->
-      <TreatWarningAsError>true</TreatWarningAsError>
+      <!--<TreatWarningAsError>true</TreatWarningAsError>-->
       <!--
         C4201: nonstandard extension used: nameless struct/union
           Conhost code uses a lot of nameless structs/unions.
@@ -150,6 +152,10 @@
       <RemoveUnreferencedCodeData>true</RemoveUnreferencedCodeData>
       <LanguageStandard>stdcpp20</LanguageStandard>
       <LanguageStandard_C>stdc17</LanguageStandard_C>
+      <!-- /fp:contract used to default to on before VS 17.0. If enabled it allows FMA for floats. -->
+      <AdditionalOptions>%(AdditionalOptions) /utf-8</AdditionalOptions>
+
+      <!-- Default build options (MSVC) -->
       <!--
         Conformance options that are off by default at the time of writing:
           /Zc:__cplusplus    Enable the __cplusplus macro to report the supported standard.
@@ -160,7 +166,16 @@
           /Zc:throwingNew    Assume operator new throws on failure.
         /diagnostics:caret   Places ^^^^^ under the exact location of the issue. The default only shows the line number.
       -->
-      <AdditionalOptions>%(AdditionalOptions) /utf-8 /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:inline /Zc:templateScope /Zc:throwingNew /diagnostics:caret</AdditionalOptions>
+      <AdditionalOptions Condition="'$(PlatformToolset)'!='ClangCL'">%(AdditionalOptions) /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:inline /Zc:templateScope /Zc:throwingNew /diagnostics:caret</AdditionalOptions>
+
+      <!-- ClangCL build options -->
+      <AdditionalOptions Condition="'$(PlatformToolset)'=='ClangCL'">%(AdditionalOptions) -Xclang -fno-rtti -D_HAS_STATIC_RTTI=0 -fno-delayed-template-parsing -march=x86-64-v3 -mcx16</AdditionalOptions>
+
+      <!-- Warning Abatement (Clang) -->
+      <AdditionalOptions Condition="'$(PlatformToolset)'=='ClangCL'">%(AdditionalOptions) -Wno-missing-braces -Wno-unused-variable -Wno-nonportable-include-path</AdditionalOptions>
+      <AdditionalOptions Condition="'$(PlatformToolset)'=='ClangCL'">%(AdditionalOptions) -Wno-microsoft-sealed -Wno-microsoft-include -Wno-pragma-pack -Wno-unused-function</AdditionalOptions>
+      <AdditionalOptions Condition="'$(PlatformToolset)'=='ClangCL'">%(AdditionalOptions) -Wno-reorder-ctor -Wno-ignored-qualifiers</AdditionalOptions>
+
       <ControlFlowGuard>Guard</ControlFlowGuard>
     </ClCompile>
     <ResourceCompile>

src/cppwinrt.build.pre.props

@@ -4,6 +4,7 @@
   <PropertyGroup>
     <!-- this will trigger the use of a project-specific out dir -->
     <OpenConsoleCppWinRTProject>true</OpenConsoleCppWinRTProject>
+    <_OpenConsoleProjectProvidedTargetPlatformIdentifier>$(TargetPlatformIdentifier)</_OpenConsoleProjectProvidedTargetPlatformIdentifier>
   </PropertyGroup>
 
   <Import Project="..\common.openconsole.props" Condition="'$(OpenConsoleDir)'==''" />
@@ -23,19 +24,11 @@
        In general, cppwinrt projects all want this.
   -->
   <PropertyGroup Condition="'$(OpenConsoleUniversalApp)'=='true'">
-    <MinimalCoreWin>true</MinimalCoreWin>
-    <AppContainerApplication>true</AppContainerApplication>
-    <WindowsStoreApp>true</WindowsStoreApp>
-    <ApplicationType>Windows Store</ApplicationType>
-    <UseCrtSDKReference Condition="'$(EnableHybridCRT)'=='true'">false</UseCrtSDKReference> <!-- The SDK reference breaks the Hybrid CRT -->
-  </PropertyGroup>
-  <PropertyGroup Condition="'$(OpenConsoleUniversalApp)'!='true'">
-    <!-- Some of our projects include the cppwinrt build options to
-         just build cppwinrt things, but don't want the store bits
-         in full swing. MinimalCoreWin != false means "don't let me
-         use win32 APIs"
-    -->
+    <!-- MinimalCoreWin=false: our binaries allowed to depend on APIs that are outside the "Store allowed" platform -->
     <MinimalCoreWin>false</MinimalCoreWin>
+    <AppContainerApplication Condition="'$(AppContainerApplication)'==''">true</AppContainerApplication>
+    <WindowsStoreApp>true</WindowsStoreApp>
+    <UseCrtSDKReference Condition="'$(EnableHybridCRT)'=='true'">false</UseCrtSDKReference> <!-- The SDK reference breaks the Hybrid CRT -->
   </PropertyGroup>
 
   <!-- This is magic that tells msbuild to link against the Desktop platform

src/host/exe/Host.EXE.vcxproj

@@ -88,6 +88,8 @@
     <Link>
       <AllowIsolation>true</AllowIsolation>
       <AdditionalDependencies>winmm.lib;imm32.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies Condition="'$(Configuration)'=='Release'">fmt.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies Condition="'$(Configuration)'=='Debug'">fmtd.lib;%(AdditionalDependencies)</AdditionalDependencies>
       <DelayLoadDLLs>icu.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
     </Link>
   </ItemDefinitionGroup>

src/host/exe/openconsole.exe.manifest

@@ -19,6 +19,7 @@
         </windowsSettings>
     </application>
 
+    <!--
     <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
         <security>
             <requestedPrivileges>
@@ -29,6 +30,7 @@
             </requestedPrivileges>
         </security>
     </trustInfo>
+    -->
     <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1"> 
         <application> 
             <!-- Windows 10 --> 

src/host/ft_host/API_BufferTests.cpp

@@ -10,6 +10,9 @@ extern "C" IMAGE_DOS_HEADER __ImageBase;
 using namespace WEX::Logging;
 using namespace WEX::Common;
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+
 // This class is intended to test boundary conditions for:
 // SetConsoleActiveScreenBuffer
 class BufferTests
@@ -294,3 +297,5 @@ void BufferTests::ChafaGifPerformance()
     const auto delta = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - now).count();
     Log::Comment(String().Format(L"%d calls took %d ms. Avg %d ms per call", count, delta, delta / count));
 }
+
+#pragma clang diagnostic pop

src/host/ft_host/API_OutputTests.cpp

@@ -9,6 +9,9 @@
 #include <vector>
 #include <algorithm>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+
 using namespace Microsoft::Console::Types;
 using namespace WEX::TestExecution;
 using namespace WEX::Logging;
@@ -1057,3 +1060,5 @@ void OutputTests::WinPtyWrite()
         break;
     }
 }
+
+#pragma clang diagnostic pop

src/host/proxy/Host.Proxy.vcxproj

@@ -24,6 +24,8 @@
     on its own.
     -->
     <EnableHybridCRT>true</EnableHybridCRT>
+    <!-- MIDL generates code clang can't link, lol -->
+    <OpenConsoleProjectRequiresMSVC>true</OpenConsoleProjectRequiresMSVC>
   </PropertyGroup>
   <Import Project="$(SolutionDir)src\common.build.pre.props" />
   <ItemGroup>

src/renderer/gdi/lib/gdi.vcxproj

@@ -23,11 +23,6 @@
     <ClInclude Include="..\gdirenderer.hpp" />
     <ClInclude Include="..\precomp.h" />
   </ItemGroup>
-  <ItemDefinitionGroup>
-    <Lib>
-      <AdditionalDependencies>usp10.lib;%(AdditionalDependencies)</AdditionalDependencies>
-    </Lib>
-  </ItemDefinitionGroup>
   <!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->
   <Import Project="$(SolutionDir)src\common.build.post.props" />
   <Import Project="$(SolutionDir)src\common.nugetversions.targets" />

src/server/ApiDispatchers.cpp

@@ -481,8 +481,6 @@ constexpr T saturate(auto val)
 [[nodiscard]] HRESULT ApiDispatchers::ServerSetConsoleActiveScreenBuffer(_Inout_ CONSOLE_API_MSG* const m,
                                                                          _Inout_ BOOL* const /*pbReplyPending*/)
 {
-    TraceConsoleAPICallWithOrigin("SetConsoleActiveScreenBuffer");
-
     const auto pObjectHandle = m->GetObjectHandle();
     RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle);
 
@@ -496,8 +494,6 @@ constexpr T saturate(auto val)
 [[nodiscard]] HRESULT ApiDispatchers::ServerFlushConsoleInputBuffer(_Inout_ CONSOLE_API_MSG* const m,
                                                                     _Inout_ BOOL* const /*pbReplyPending*/)
 {
-    TraceConsoleAPICallWithOrigin("ServerFlushConsoleInputBuffer");
-
     const auto pObjectHandle = m->GetObjectHandle();
     RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle);
 

src/server/lib/server.vcxproj

@@ -59,9 +59,9 @@
     <ClInclude Include="..\WinNTControl.h" />
   </ItemGroup>
   <ItemGroup>
-    <ProjectReference Include="..\..\host\proxy\Host.Proxy.vcxproj">
+	  <!--<ProjectReference Include="..\..\host\proxy\Host.Proxy.vcxproj">
       <Project>{71CC9D78-BA29-4D93-946F-BEF5D9A3A6EF}</Project>
-    </ProjectReference>
+    </ProjectReference>-->
   </ItemGroup>
   <ItemDefinitionGroup>
     <ClCompile>

src/tools/ConsoleBench/arena.cpp

@@ -83,7 +83,7 @@ ScopedArena::~ScopedArena()
     arena.pop_to(m_pos_backup);
 }
 
-static [[msvc::noinline]] std::array<Arena, 2> thread_arenas_init()
+[[msvc::noinline]] static std::array<Arena, 2> thread_arenas_init()
 {
     return {
         Arena{ 1024 * 1024 * 1024 },