Skip to content

修复 hash_combine 中最后一个参数被忽略的 Bug#515

Open
yuanjay07 wants to merge 1 commit into
BoomingTech:mainfrom
yuanjay07:fix-hash-combine
Open

修复 hash_combine 中最后一个参数被忽略的 Bug#515
yuanjay07 wants to merge 1 commit into
BoomingTech:mainfrom
yuanjay07:fix-hash-combine

Conversation

@yuanjay07

Copy link
Copy Markdown

修复的问题描述

engine/source/runtime/core/base/hash.h 中,多参数版本的 hash_combine 函数实现如下:

template<typename T, typename... Ts>
inline void hash_combine(std::size_t& seed, const T& v, Ts... rest)
{
    hash_combine(seed, v);
    if constexpr (sizeof...(Ts) > 1)
    {
        hash_combine(seed, rest...);
    }
}

递归的终止条件检查为 sizeof...(Ts) > 1。这会导致一个严重的 Bug:

  • 当递归到只剩最后一个变参(即 sizeof...(Ts) 的值为 1)时,条件判断 1 > 1 的结果为 false
  • 这导致递归提前结束,最后一个参数完全没有通过 hash_combine 合并到 seed 中,从而被静默忽略。

具体场景举例

MaterialSourceDesc::getHashValue() 中,哈希计算传入了 5 个参数:

hash_combine(hash,
             m_base_color_file,
             m_metallic_roughness_file,
             m_normal_file,
             m_occlusion_file,
             m_emissive_file);

在递归处理到第 4 个参数 m_occlusion_file 时,剩余的变参包 Ts... 中只包含 m_emissive_file(此时 sizeof...(Ts) 为 1)。
由于 1 > 1 为假,m_emissive_file 不会被递归调用,导致最终算出的 hash 根本没有包含 m_emissive_file 的哈希值。


解决方案

将递归终止条件修改为 sizeof...(Ts) > 0

template<typename T, typename... Ts>
inline void hash_combine(std::size_t& seed, const T& v, Ts... rest)
{
    hash_combine(seed, v);
    if constexpr (sizeof...(Ts) > 0)
    {
        hash_combine(seed, rest...);
    }
}

改进后的逻辑

  1. sizeof...(Ts) 大于 0(即还有剩余参数)时,条件继续成立,调用 hash_combine(seed, rest...)
  2. 当只剩下 1 个参数时,sizeof...(Ts) 为 1,条件 1 > 0true,触发调用 hash_combine(seed, rest...)(此时只有两个实际参数:seed 和最后一个元素)。
  3. 编译器在重载解析时,会匹配到特化程度更高的双参数非模板重载(或者说非变参模板重载):
    template<typename T>
    inline void hash_combine(std::size_t& seed, const T& v)
    从而正确合并最后一个参数并终止递归。

影响范围

  • 修复了所有使用 hash_combine 处理多参数时的潜在哈希冲突与不准确性,保证了哈希合并的完整性。
  • 对编译期效率或运行时开销无额外负面影响(均在编译期实例化时完成展开)。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant