philnik added a comment. In D147175#4251076 <https://reviews.llvm.org/D147175#4251076>, @aaron.ballman wrote:
> One thing I can't quite determine is whether we expect to call this type > trait often or not. Are either of the uses in libc++ going to be instantiated > frequently? I'm trying to figure out whether we want the current > implementation of `HasNonDeletedDefaultedEqualityComparison()` or whether we > should bite the bullet up front and add another bit to > `CXXRecordDecl::DefinitionData` so we compute this property once as the class > is being formed and don't have to loop over all of the methods and bases each > time. We do this for other special member functions, as with: > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclCXX.h#L695 We currently use `__is_trivially_equality_comparable` only for `std::equal` (and I'm working on using it for `std::find`), and I don't think there are a lot more places where we can use it. It's also only relevant for cases where we have raw pointers. Would the `enable_if` be evaluated, even if the arguments aren't raw pointers in the example below? template <class T, class U, enable_if_t<is_same_v<remove_cvref_t<T>, remove_cvref_t<U>> && __is_trivially_equality_comparable(T)>> bool equal(T* first1, T* last1, U* first2); <non-raw-pointer-overload> If not, then I don't think it makes sense to save the information, since it will most likely not be evaluated more than once or twice per type. ================ Comment at: clang/lib/AST/Type.cpp:2618-2619 + llvm::all_of(Decl->fields(), [](const FieldDecl *FD) { + if (!FD->getType()->isRecordType()) + return !FD->getType()->isReferenceType(); + return HasNonDeletedDefaultedEqualityComparison( ---------------- aaron.ballman wrote: > Why the check for !RecordType? If any field is a reference, we can bail out > early, so I think this is better as: > ``` > if (FD->getType()->isReferenceType()) > return false; > if (const auto *RD = FD->getType()->getAsCXXRecordDecl()) > return HasNonDeletedDefaultedEqualityComparison(RD); > return true; > ``` > WDYT? I don't really have an opinion here. Your version seems reasonable, so I'm going to take it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147175/new/ https://reviews.llvm.org/D147175 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
