Skip to content

Commit

Permalink
Address variable name collisions in generated consume methods (#1455)
Browse files Browse the repository at this point in the history
Why is this change being made?
Someone discovered a subtle compatibility issue with the cast result checking changes and certain projected methods. The variables in the consume_ method, such as the generically-named code variable, can shadow the parameters to the function. This led to a build break where a function took an int16_t named code and that was shadowed by the int32_t named code with the HRESULT in it. Fortunately the compiler had our back and flagged the size truncation as a build break so it was noticed.

Briefly summarize what changed
The variable names in these generated functions are now much uglier (and therefore less likely to collide by coincidence). They have an underscore prefix and then lowercase which should put them into an unofficial namespace that shouldn't collide with anything else. The code variable is now named _winrt_cast_result_code for example. While I was renaming everything I realized that my previous names mismatched the cppwinrt naming convention of snake_case so I fixed them up.
  • Loading branch information
dmachaj authored Nov 20, 2024
1 parent 2aed347 commit fa079fb
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions cppwinrt/code_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -1137,15 +1137,15 @@ namespace cppwinrt
{%
if constexpr (!std::is_same_v<D, %>)
{
auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(code);
auto const abiType = *(abi_t<%>**)&castedResult;
abiType->%(%);
auto const [_winrt_casted_result, _winrt_cast_result_code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(_winrt_cast_result_code);
auto const _winrt_abi_type = *(abi_t<%>**)&_winrt_casted_result;
_winrt_abi_type->%(%);
}
else
{
auto const abiType = *(abi_t<%>**)this;
abiType->%(%);
auto const _winrt_abi_type = *(abi_t<%>**)this;
_winrt_abi_type->%(%);
}%
}
)";
Expand All @@ -1156,15 +1156,15 @@ namespace cppwinrt
{%
if constexpr (!std::is_same_v<D, %>)
{
auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(code);
auto const abiType = *(abi_t<%>**)&castedResult;
WINRT_VERIFY_(0, abiType->%(%));
auto const [_winrt_casted_result, _winrt_cast_result_code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(_winrt_cast_result_code);
auto const _winrt_abi_type = *(abi_t<%>**)&_winrt_casted_result;
WINRT_VERIFY_(0, _winrt_abi_type->%(%));
}
else
{
auto const abiType = *(abi_t<%>**)this;
WINRT_VERIFY_(0, abiType->%(%));
auto const _winrt_abi_type = *(abi_t<%>**)this;
WINRT_VERIFY_(0, _winrt_abi_type->%(%));
}%
}
)";
Expand All @@ -1176,15 +1176,15 @@ namespace cppwinrt
{%
if constexpr (!std::is_same_v<D, %>)
{
auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(code);
auto const abiType = *(abi_t<%>**)&castedResult;
check_hresult(abiType->%(%));
auto const [_winrt_casted_result, _winrt_cast_result_code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
check_hresult(_winrt_cast_result_code);
auto const _winrt_abi_type = *(abi_t<%>**)&_winrt_casted_result;
check_hresult(_winrt_abi_type->%(%));
}
else
{
auto const abiType = *(abi_t<%>**)this;
check_hresult(abiType->%(%));
auto const _winrt_abi_type = *(abi_t<%>**)this;
check_hresult(_winrt_abi_type->%(%));
}%
}
)";
Expand Down

0 comments on commit fa079fb

Please sign in to comment.