Hide protected and overridable members from public projections (#1319)

Author: sylveon

README.md

@@ -15,6 +15,16 @@ If you really want to build it yourself, the simplest way to do so is to run the
 
 * Open a dev command prompt pointing at the root of the repo.
 * Open the `cppwinrt.sln` solution.
-* Build the x64 Release configuration of the `prebuild` and `cppwinrt` projects only. Do not attempt to build anything else just yet.
+* Build the x64 Release configuration of the `cppwinrt` project only. Do not attempt to build anything else just yet.
 * Run `build_projection.cmd` in the dev command prompt.
 * Switch to the x64 Debug configuration in Visual Studio and build all projects as needed.
+
+## Comparing Outputs
+
+Comparing the output of the prior release and your current changes will help show the impact of any updates. Starting from
+a dev command prompt at the root of the repo _after_ following the above build instructions:
+
+* Run `build_projection.cmd` in the dev command prompt
+* Run `build_prior_projection.cmd` in the dev command prompt as well
+* Run `prepare_versionless_diffs.cmd` which removes version stamps on both current and prior projection
+* Use a directory-level differencing tool to compare `_build\$(arch)\$(flavor)\winrt` and `_reference\$(arch)\$(flavor)\winrt`

build_prior_projection.cmd

@@ -0,0 +1,50 @@
+@echo off
+
+setlocal ENABLEDELAYEDEXPANSION
+
+set target_platform=%1
+set target_configuration=%2
+if "%target_platform%"=="" set target_platform=x64
+
+if /I "%target_platform%" equ "all" (
+  if "%target_configuration%"=="" (
+    set target_configuration=all
+  )
+  call %0 x86 !target_configuration!
+  call %0 x64 !target_configuration!
+  call %0 arm !target_configuration!
+  call %0 arm64 !target_configuration!
+  goto :eof
+)
+
+if /I "%target_configuration%" equ "all" (
+  call %0 %target_platform% Debug
+  call %0 %target_platform% Release
+  goto :eof
+)
+
+if "%target_configuration%"=="" (
+ set target_configuration=Debug
+)
+
+set reference_output=%~p0\_reference\%target_platform%\%target_configuration%
+if exist "%reference_output%" (
+ echo Removing existing reference projections
+ rmdir /s /q "%reference_output%"
+)
+
+if not exist ".\.nuget" mkdir ".\.nuget"
+if not exist ".\.nuget\nuget.exe" powershell -Command "$ProgressPreference = 'SilentlyContinue' ; Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -OutFile .\.nuget\nuget.exe"
+
+mkdir %reference_output%\package
+.\.nuget\nuget.exe install Microsoft.Windows.CppWinRT -o %reference_output%\package
+set reference_cppwinrt=
+for /F "delims=" %%a in ('dir /s /b %reference_output%\package\cppwinrt.exe') DO set reference_cppwinrt=%%a
+if "%reference_cppwinrt%"=="" (
+ echo Could not find the reference cppwinrt.exe under %reference_output%\package
+ goto :EOF
+)
+
+echo Generating reference projection from %reference_cppwinrt% to %reference_output%\cppwinrt
+%reference_cppwinrt% -in local -out %reference_output% -verbose
+echo.

build_projection.cmd

@@ -27,6 +27,13 @@ if "%target_configuration%"=="" (
  set target_configuration=Debug
 )
 
+set cppwinrt_exe=%~p0\_build\x64\Release\cppwinrt.exe
+
+if not exist "%cppwinrt_exe%" (
+ echo Remember to build the "prebuild" and then "cppwinrt" projects for Release x64 first
+ goto :eof
+)
+
 echo Building projection into %target_platform% %target_configuration%
-%~p0\_build\x64\Release\cppwinrt.exe -in local -out %~p0\_build\%target_platform%\%target_configuration% -verbose
+%cppwinrt_exe% -in local -out %~p0\_build\%target_platform%\%target_configuration% -verbose
 echo.

build_test_all.cmd

@@ -10,7 +10,7 @@ if "%target_configuration%"=="" set target_configuration=Release
 if "%target_version%"=="" set target_version=1.2.3.4
 
 if not exist ".\.nuget" mkdir ".\.nuget"
-if not exist ".\.nuget\nuget.exe" powershell -Command "Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -OutFile .\.nuget\nuget.exe"
+if not exist ".\.nuget\nuget.exe" powershell -Command "$ProgressPreference = 'SilentlyContinue' ; Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -OutFile .\.nuget\nuget.exe"
 
 call .nuget\nuget.exe restore cppwinrt.sln"
 call .nuget\nuget.exe restore natvis\cppwinrtvisualizer.sln

cppwinrt/code_writers.h

@@ -2035,13 +2035,39 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
     {
         for (auto&& [name, info] : interfaces)
         {
-            if (!info.overridable)
+            if (!info.overridable && !info.is_protected)
             {
                 w.write(", %", name);
             }
         }
     }
 
+    static void write_class_override_protected_requires(writer& w, get_interfaces_t const& interfaces)
+    {
+        bool first = true;
+
+        for (auto&& [name, info] : interfaces)
+        {
+            if (info.is_protected)
+            {
+                if (first)
+                {
+                    first = false;
+                    w.write(",\n        protected impl::require<D, %", name);
+                }
+                else
+                {
+                    w.write(", %", name);
+                }
+            }
+        }
+
+        if (!first)
+        {
+            w.write('>');
+        }
+    }
+
     static void write_class_override_defaults(writer& w, get_interfaces_t const& interfaces)
     {
         bool first = true;
@@ -2073,6 +2099,18 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
         }
     }
 
+    static void write_class_override_friends(writer& w, get_interfaces_t const& interfaces)
+    {
+        for (auto&& [name, info] : interfaces)
+        {
+            if (info.is_protected)
+            {
+                w.write("\n        friend impl::consume_t<D, %>;", name);
+                w.write("\n        friend impl::require_one<D, %>;", name);
+            }
+        }
+    }
+
     static void write_call_factory(writer& w, TypeDef const& type, TypeDef const& factory)
     {
         std::string factory_name;
@@ -2243,10 +2281,10 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
         auto format = R"(    template <typename D, typename... Interfaces>
     struct %T :
         implements<D%, composing, Interfaces...>,
-        impl::require<D%>,
+        impl::require<D%>%,
         impl::base<D, %%>%
     {
-        using composable = %;
+        using composable = %;%
     protected:
 %%    };
 )";
@@ -2258,10 +2296,12 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
             type_name,
             bind<write_class_override_implements>(interfaces),
             bind<write_class_override_requires>(interfaces),
+            bind<write_class_override_protected_requires>(interfaces),
             type_name,
             bind<write_class_override_bases>(type),
             bind<write_class_override_defaults>(interfaces),
             type_name,
+            bind<write_class_override_friends>(interfaces),
             bind<write_class_override_constructors>(type, factories),
             bind<write_class_override_usings>(interfaces));
     }
@@ -2326,18 +2366,21 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
 
         for (auto&& [interface_name, info] : get_interfaces(w, type))
         {
-            if (info.defaulted && !info.base)
+            if (!info.is_protected && !info.overridable)
             {
-                for (auto&& method : info.type.MethodList())
+                if (info.defaulted && !info.base)
                 {
-                    method_usage[get_name(method)].insert(default_interface_name);
+                    for (auto&& method : info.type.MethodList())
+                    {
+                        method_usage[get_name(method)].insert(default_interface_name);
+                    }
                 }
-            }
-            else
-            {
-                for (auto&& method : info.type.MethodList())
+                else
                 {
-                    method_usage[get_name(method)].insert(interface_name);
+                    for (auto&& method : info.type.MethodList())
+                    {
+                        method_usage[get_name(method)].insert(interface_name);
+                    }
                 }
             }
         }
@@ -2804,7 +2847,7 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
 
         for (auto&& [interface_name, info] : get_interfaces(w, type))
         {
-            if (!info.defaulted || info.base)
+            if ((!info.defaulted || info.base) && (!info.is_protected && !info.overridable))
             {
                 if (first)
                 {

cppwinrt/component_writers.h

@@ -743,13 +743,13 @@ catch (...) { return winrt::to_hresult(); }
             auto format = R"(namespace winrt::@::implementation
 {
     template <typename D%, typename... I>
-    struct WINRT_IMPL_EMPTY_BASES %_base : implements<D, @::%%%, %I...>%%%
+    struct WINRT_IMPL_EMPTY_BASES %_base : implements<D, @::%%%, %I...>%%%%
     {
         using base_type = %_base;
         using class_type = @::%;
         using implements_type = typename %_base::implements_type;
         using implements_type::implements_type;
-        %
+        %%
         hstring GetRuntimeClassName() const
         {
             return L"%.%";
@@ -764,6 +764,8 @@ catch (...) { return winrt::to_hresult(); }
             std::string base_type_argument;
             std::string no_module_lock;
             std::string external_requires;
+            std::string external_protected_requires;
+            std::string friends;
 
             if (base_type)
             {
@@ -774,7 +776,9 @@ catch (...) { return winrt::to_hresult(); }
                     composable_base_name = w.write_temp("using composable_base = %;", base_type);
                     auto base_interfaces = get_interfaces(w, base_type);
                     uint32_t base_interfaces_count{};
+                    uint32_t protected_base_interfaces_count{};
                     external_requires = ",\n        impl::require<D";
+                    external_protected_requires = ",\n        protected impl::require<D";
 
                     for (auto&&[name, info] : base_interfaces)
                     {
@@ -783,9 +787,25 @@ catch (...) { return winrt::to_hresult(); }
                             continue;
                         }
 
-                        ++base_interfaces_count;
-                        external_requires += ", ";
-                        external_requires += name;
+                        if (info.is_protected || info.overridable)
+                        {
+                            ++protected_base_interfaces_count;
+                            external_protected_requires += ", ";
+                            external_protected_requires += name;
+
+                            friends += "\n        friend impl::consume_t<D, ";
+                            friends += name;
+                            friends += ">;";
+                            friends += "\n        friend impl::require_one<D, ";
+                            friends += name;
+                            friends += ">;";
+                        }
+                        else
+                        {
+                            ++base_interfaces_count;
+                            external_requires += ", ";
+                            external_requires += name;
+                        }
                     }
 
                     if (base_interfaces_count)
@@ -796,6 +816,15 @@ catch (...) { return winrt::to_hresult(); }
                     {
                         external_requires.clear();
                     }
+
+                    if (protected_base_interfaces_count)
+                    {
+                        external_protected_requires += '>';
+                    }
+                    else
+                    {
+                        external_protected_requires.clear();
+                    }
                 }
                 else
                 {
@@ -816,13 +845,15 @@ catch (...) { return winrt::to_hresult(); }
                 base_type_argument,
                 no_module_lock,
                 external_requires,
+                external_protected_requires,
                 bind<write_component_class_base>(type),
                 bind<write_component_override_defaults>(type),
                 type_name,
                 type_namespace,
                 type_name,
                 type_name,
                 composable_base_name,
+                friends,
                 type_namespace,
                 type_name,
                 bind<write_component_class_override_constructors>(type),

cppwinrt/helpers.h

@@ -527,6 +527,7 @@ namespace cppwinrt
     {
         TypeDef type;
         bool is_default{};
+        bool is_protected{};
         bool defaulted{};
         bool overridable{};
         bool base{};
@@ -577,6 +578,7 @@ namespace cppwinrt
             auto type = impl.Interface();
             auto name = w.write_temp("%", type);
             info.is_default = has_attribute(impl, "Windows.Foundation.Metadata", "DefaultAttribute");
+            info.is_protected = has_attribute(impl, "Windows.Foundation.Metadata", "ProtectedAttribute");
             info.defaulted = !base && (defaulted || info.is_default);
 
             {

prepare_versionless_diffs.cmd

@@ -0,0 +1,41 @@
+@echo off
+
+setlocal ENABLEDELAYEDEXPANSION
+
+set target_platform=%1
+set target_configuration=%2
+if "%target_platform%"=="" set target_platform=x64
+
+if /I "%target_platform%" equ "all" (
+  if "%target_configuration%"=="" (
+    set target_configuration=all
+  )
+  call %0 x86 !target_configuration!
+  call %0 x64 !target_configuration!
+  call %0 arm !target_configuration!
+  call %0 arm64 !target_configuration!
+  goto :eof
+)
+
+if /I "%target_configuration%" equ "all" (
+  call %0 %target_platform% Debug
+  call %0 %target_platform% Release
+  goto :eof
+)
+
+if "%target_configuration%"=="" (
+ set target_configuration=Debug
+)
+
+set reference_output=%~p0\_reference\%target_platform%\%target_configuration%
+set build_output=%~p0\_build\%target_platform%\%target_configuration%
+
+echo Removing version stamps from %reference_output%\winrt
+pushd %reference_output%\winrt
+powershell -Command "gci -r -include *.h,*.ixx | %%{ (get-content $_) -replace 'was generated by.*|CPPWINRT_VERSION.*','' | set-content $_ }"
+popd
+
+echo Removing version stamps from %build_output%\winrt
+pushd %build_output%\winrt
+powershell -Command "gci -r -include *.h,*.ixx | %%{ (get-content $_) -replace 'was generated by.*|CPPWINRT_VERSION.*','' | set-content $_ }"
+popd

test/old_tests/Composable/Base.cpp

@@ -41,6 +41,11 @@ namespace winrt::Composable::implementation
         return 42;
     }
 
+    int32_t Base::ProtectedMethod()
+    {
+        return 0xDEADBEEF;
+    }
+
     hstring Base::Name() const
     {
         return m_name;

test/old_tests/Composable/Base.h

@@ -18,6 +18,7 @@ namespace winrt::Composable::implementation
         hstring OverridableMethod() ;
         virtual hstring OverridableVirtualMethod();
         int32_t OverridableNoexceptMethod() noexcept;
+        int32_t ProtectedMethod();
 
         hstring Name() const;