Quuxplusone / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
https://p1144.godbolt.org/z/jf67zx5hq
Other
0 stars 2 forks source link

`__is_trivially_equality_comparable` returns false for enum types such as `std::byte` #36

Open Quuxplusone opened 3 months ago

Quuxplusone commented 3 months ago

https://godbolt.org/z/3W9ohKhza

#include <memory>
static_assert(__is_trivially_equality_comparable(std::byte));

currently fails (for both libc++ and libstdc++). This is because Clang invariably reports false for enum types. Back when __is_trivially_equality_comparable was first implemented, we made it conservatively return false because you might have

enum E { RED };
static_assert(__is_trivially_equality_comparable(E));
bool operator==(E, E);
static_assert(!__is_trivially_equality_comparable(E)); // ha ha, ODR violation!

but these days we've accepted that exact risk for class types, e.g.

struct S { int i; friend bool operator==(S, S); };
static_assert(!__is_trivially_equality_comparable(S));
bool operator==(S, S) = default;
static_assert(__is_trivially_equality_comparable(S)); // ha ha, ODR violation!

so we might as well accept it for enum types too (where the risk is astronomically less in practice, anyway: I think I've never seen an enum type with overloaded comparison operators).

Implementing __is_trivially_equality_comparable for enum types will eliminate a pessimization that libc++ currently has for e.g. std::equal<std::byte> versus std::equal<char>. https://godbolt.org/z/TcbjPbfPs

changkhothuychung commented 1 month ago

Hi @Quuxplusone , just curious, for your second code snippet, why is there a ODR violation in the second static_assert? Which name is declared more than once in this case?

Quuxplusone commented 1 month ago

@changkhothuychung: Well, "ODR violation" here is short for "you can build an ODR violation based on this." Any time you have a template (or other constexpr expression) whose value changes over the course of the translation unit, you can build an ODR violation of the form:

// odr.hpp
inline constexpr bool violate() { return __is_trivially_equality_comparable(S); }
inline bool boom() { return []() { return std::bool_constant<violate()>(); }(); }

// s.hpp
inline bool operator==(S, S) = default;

// tu1.cpp
struct S { int i; friend bool operator==(S, S); };
#include <odr.hpp>
[...]

// tu2.cpp
struct S { int i; friend bool operator==(S, S); };
#include <s.hpp>
#include <odr.hpp>
[...]

Now tu1.o:violate and tu2.o:violate are inline functions with the same name and signature (i.e. the same function), but with two different runtime behaviors. That's an ODR violation — or, well, maybe we're still speaking colloquially in that case. To be 100% formal about it, tu1.o:boom and tu2.o:boom are two definitions of the same inline function, using the same sequence of tokens (basic.def.odr/15.4), but in which corresponding lambda-expressions — consisting of the tokens []() { return std::bool_constant<violate()>(); }() — do not have the same closure type (violating basic.def.odr/15.6).