[libcxx-dev] Is the implementation of two-argument __non_unique_arm_rtti_bit_impl; ; __is_type_name_unique correct?

[Stephan Bergmann via libcxx-dev]

On 02/03/2021 22:25, Louis Dionne wrote:
>> On Dec 16, 2020, at 09:07, Stephan Bergmann via libcxx-dev 
>> <libcxx-dev at lists.llvm.org <mailto:libcxx-dev at lists.llvm.org>> wrote:
>>
>> About the code originally introduced into libcxx/include/typeinfo as
>>
>>>    _LIBCPP_INLINE_VISIBILITY
>>>    bool operator==(const type_info& __arg) const _NOEXCEPT
>>> #ifndef _LIBCPP_NONUNIQUE_RTTI_BIT
>>>        {return __type_name == __arg.__type_name;}
>>> #else
>>>        {if (__type_name == __arg.__type_name) return true;
>>>         if (!((__type_name & __arg.__type_name) & 
>>> _LIBCPP_NONUNIQUE_RTTI_BIT))
>>>           return false;
>>>         return __compare_nonunique_names(__arg) == 0;}
>>> #endif
>>
>> in 
>> <https://github.com/llvm/llvm-project/commit/0090e657cb3a477ace4db59a6b5ae80baffec4c5 
>> <https://github.com/llvm/llvm-project/commit/0090e657cb3a477ace4db59a6b5ae80baffec4c5>> 
>> "ARM64: compare RTTI names as strings", and then factored out to
>>
>>>    _LIBCPP_INLINE_VISIBILITY
>>>    static bool __is_type_name_unique(__type_name_t __lhs, 
>>> __type_name_t __rhs) _NOEXCEPT {
>>>      return !((__lhs & __rhs) & __non_unique_rtti_bit::value);
>>>    }
>>
>> in 
>> <https://github.com/llvm/llvm-project/commit/2405bd6898151e0a7ffede78b0d0c7c85c0b66d3 
>> <https://github.com/llvm/llvm-project/commit/2405bd6898151e0a7ffede78b0d0c7c85c0b66d3>> 
>> "Rework std::type_info definition to support systems without fully 
>> merged type info names":
>>
>> I wonder if it is correct to compute `__lhs & __rhs` rather than 
>> `__lhs | __rhs`?  The documentation of NonUniqueARMRTTIBit (also in 
>> libcxx/include/typeinfo) states that "we check whether BOTH type_infos 
>> are guaranteed unique, and if so, we simply compare the addresses of 
>> their type names instead of doing a deep string comparison, which is 
>> faster.  If at least one of the type_infos can't guarantee uniqueness, 
>> we have no choice but to fall back to a deep string comparison."
>>
>> So my understanding is that __is_type_name_unique should return false 
>> (and comparison fall back to deep string comparison) when at least one 
>> of __lhs and __rhs has the __non_unique_rtti_bit set, not only when 
>> both have it set.
> 
> Yes, I do follow your thinking and I think you’re right. I went ahead 
> and created a review (https://reviews.llvm.org/D97802 
> <https://reviews.llvm.org/D97802>) to avoid forgetting about this. I’ll 
> need to investigate some more to make sure your interpretation is correct.

I now remember what prompted me to write the original mail, see the 
commit message of 
<https://git.libreoffice.org/core/+/bf858e4b224ae4dc04f968a3d3c66d184dd1e33d%5E!/> 
"Fix macOS ARM64 RTTI generation":  LibreOffice generates fake RTTI data 
at runtime, but had forgotten to set the NonUniqueARMRTTIBit.  And what 
had puzzled me was that that fake RTTI (with NonUniqueARMRTTIBit unset) 
failed to compare equal to the real RTTI emitted by Clang (with 
NonUniqueARMRTTIBit set), given the documentation of NonUniqueARMRTTIBit 
in libcxx/include/typeinfo quoted above.

But <https://reviews.llvm.org/D97802> "[libc++] Fix incorrect typeinfo 
comparison on ARM64" now clarifies that it was the documentation that 
was actually wrong.  Thanks for having picked this up!