Skip to content

Hide protected and overridable members from public projections #1319

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

Merged
merged 10 commits into from
Jul 12, 2023
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
12 changes: 11 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
50 changes: 50 additions & 0 deletions build_prior_projection.cmd
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 8 additions & 1 deletion build_projection.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion build_test_all.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 55 additions & 12 deletions cppwinrt/code_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
%% };
)";
Expand All @@ -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));
}
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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)
{
Expand Down
41 changes: 36 additions & 5 deletions cppwinrt/component_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"%.%";
Expand All @@ -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)
{
Expand All @@ -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)
{
Expand All @@ -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)
Expand All @@ -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
{
Expand All @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions cppwinrt/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ namespace cppwinrt
{
TypeDef type;
bool is_default{};
bool is_protected{};
bool defaulted{};
bool overridable{};
bool base{};
Expand Down Expand Up @@ -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);

{
Expand Down
41 changes: 41 additions & 0 deletions prepare_versionless_diffs.cmd
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions test/old_tests/Composable/Base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ namespace winrt::Composable::implementation
return 42;
}

int32_t Base::ProtectedMethod()
{
return 0xDEADBEEF;
}

hstring Base::Name() const
{
return m_name;
Expand Down
1 change: 1 addition & 0 deletions test/old_tests/Composable/Base.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace winrt::Composable::implementation
hstring OverridableMethod() ;
virtual hstring OverridableVirtualMethod();
int32_t OverridableNoexceptMethod() noexcept;
int32_t ProtectedMethod();

hstring Name() const;

Expand Down
Loading