philnik added a comment. In D147175#4246513 <https://reviews.llvm.org/D147175#4246513>, @aaron.ballman wrote:
> Ah, I like this approach -- it keeps things roughly in sync with checking for > unique object representations, which is great. I spotted a few questions, but > in general this is heading in the right direction. You should also add a > release note to clang/docs/ReleaseNotes.rst so that users know there's a new > builtin coming. > > I suppose one thing to double-check: do you know of any other STL > implementations that might be using this identifier as part of their > implementation details? (We've had issues in the past where we accidentally > stole a reserved identifier that was being used by libstdc++ and it caused > issues.) Neither libstdc++ nor the MSVC STL use the `__is_trivially_equality_comparable` AFAICT. A sourcegraph search also exclusively finds libc++: https://sourcegraph.com/search?q=context%3Aglobal+__is_trivially_equality_comparable&patternType=standard&sm=1&groupBy=repo (which only started using it in this release cycle, so it shouldn't be a problem). ================ Comment at: clang/lib/AST/Type.cpp:2598-2599 +static bool HasDefaultedEqualityComparison(const CXXRecordDecl *Decl) { + if (Decl->isUnion()) + return false; + ---------------- aaron.ballman wrote: > Hmmm, is this correct? I thought there was a defaulted equality comparison > operator in this case, but it's defined as deleted. > > http://eel.is/c++draft/class.compare.default#2 > > Perhaps this function is looking for a usable defaulted equality comparison > operator and not just "does it have one at all"? Yes, this is looking for a usable one. I've renamed it. (Maybe someone has an idea for a better name?) ================ Comment at: clang/lib/AST/Type.cpp:2616-2617 + }) && + llvm::all_of(Decl->fields(), [](const FieldDecl *FD) { + if (!FD->getType()->isRecordType()) + return true; ---------------- aaron.ballman wrote: > Do we have to look for fields with references per > http://eel.is/c++draft/class.compare.default#2 ? Good catch! 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
