[Bug libstdc++/103240] std::type_info::before gives wrong answer for ARM EABI

["redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103240

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to frankhb1989 from comment #5)
> There are multiple issues.
> 
> 1. The out-of-line definition and the inline definition of
> std::type_info::before do different things. Specifically, only one name is
> detected against to '*' in the former, but two in the latter. It seems the
> latter is more correct. ("More", because it is still not quite correct, see
> below.)

I think the latter is completely correct.

> 2. As stated in https://reviews.llvm.org/D100134, mixture of different
> methods of comparisons breaks the ordering.

I don't think that's a problem for our inline implementation.

For non-unique A, unique B, non-unique C we would always do string comparisons,
and we would consistently have "*B" < "A" < "C" in all TUs.

We would only do pointer comparisons for when comparing two unique types, e.g.
comparing "*B" and "*B" or comparing "*B" and "*D".

> I've actually encountered the problem with std::type_index as the key type
> when using flat_map::emplace (like
> https://github.com/WG21-SG14/SG14/blob/master/SG14/flat_map.h, which uses
> std::partition_point to find the position of insertion). The insertion
> suddenly fails when std::partition_point meets two std::type_info objects x
> and y which are unordered. It works with the workaround
> '-U__GXX_MERGED_TYPEINFO_NAMES -D__GXX_MERGED_TYPEINFO_NAMES=1' in the
> compiler command line, but it seems not enough in general without fixing the
> issue 2 here. (Although the same sequence of key on std::map does not fail,
> occasionally, due to less comparisons are made.)

What are the mangled type names that are unordered? (that's all you need to
describe, everything about flat_map and partition_point is irrelevant; just
tell us which names are unordered).

Is this using the inline implementation? (I assume so, otherwise redefining
__GXX_MERGED_TYPEINFO_NAMES won't do anything). How does forcing address
comparisons for all types solve anything? That isn't just "not enough in
general" it's **wrong** in general.

> 3. Not sure the original change in r179236 is correct.

It was an improvement, at least.

> 4. '||' in the condition of the inline definition of std::type_info::before
> seems an overkill. '&&' may be enough. Assuming string comparison is more
> expensive, this is a simple optimization. But once the issue 2 is fixed, the
> change would look quite different.

I don't think that's correct. We can only do a pointer comparison if both
strings begin with '*'. If we compared pointers for "*foo" and "bar" (where
"bar" is not unique), we might have "*foo" < "bar"  in one TU and "*foo" >
"bar" in another TU where "bar" has a different address. We need to do a string
comparison if either of them is not unique. Which is what the inline
implementation does.

It also correctly handles the problem case described by Richard Smith, because
all unique names start with '*' and all non-unique names start with one of
[A-Za-z_], and those are ordered after '*', at least for ASCII and UTF-8.

I agree there are issues with the non-inline implementation. We could just make
it match the inline one:

--- a/libstdc++-v3/libsupc++/tinfo2.cc
+++ b/libstdc++-v3/libsupc++/tinfo2.cc
@@ -33,15 +33,11 @@ using std::type_info;
 bool
 type_info::before (const type_info &arg) const _GLIBCXX_NOEXCEPT
 {
-#if __GXX_MERGED_TYPEINFO_NAMES
-  return name () < arg.name ();
-#else
-  /* The name() method will strip any leading '*' prefix. Therefore
-     take care to look at __name rather than name() when looking for
-     the "pointer" prefix.  */
-  return (__name[0] == '*') ? name () < arg.name ()
-    :  __builtin_strcmp (name (), arg.name ()) < 0;
+#if !__GXX_MERGED_TYPEINFO_NAMES
+  if (__name[0] == '*' || arg.__name[0] == '*')
+    return __builtin_strcmp (__name, arg.__name) < 0;
 #endif
+  return __name < arg.__name
 }

 #endif