On Thu, Dec 17, 2015 at 6:25 PM, Richard Trieu <rtr...@google.com> wrote: > The assert was the one later in the function: > assert(getMemoizationData() && Other.getMemoizationData()); > My best guess as to what happened was my configuration had a container which > reordered the comparison, making the QualType comparison happen earlier than > it would otherwise. > r255937 has a small test case which demonstrates where the assert used to > happen.
Thanks! ~Aaron > > On Thu, Dec 17, 2015 at 2:05 PM, Aaron Ballman <aa...@aaronballman.com> > wrote: >> >> On Thu, Dec 17, 2015 at 5:02 PM, Richard Trieu <rtr...@google.com> wrote: >> > On Thu, Dec 17, 2015 at 5:47 AM, Aaron Ballman <aa...@aaronballman.com> >> > wrote: >> >> >> >> On Wed, Dec 16, 2015 at 11:46 PM, Richard Trieu via cfe-commits >> >> <cfe-commits@lists.llvm.org> wrote: >> >> > Author: rtrieu >> >> > Date: Wed Dec 16 22:46:48 2015 >> >> > New Revision: 255875 >> >> > >> >> > URL: http://llvm.org/viewvc/llvm-project?rev=255875&view=rev >> >> > Log: >> >> > Add QualType case to operator< for DynTypedNode. >> >> > >> >> > This allows sorting DynTypedNode's which are QualType's since >> >> > QualType >> >> > does >> >> > not have memoization. >> >> > >> >> > Modified: >> >> > cfe/trunk/include/clang/AST/ASTTypeTraits.h >> >> > >> >> > Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h >> >> > URL: >> >> > >> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=255875&r1=255874&r2=255875&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original) >> >> > +++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Wed Dec 16 22:46:48 >> >> > 2015 >> >> > @@ -271,6 +271,10 @@ public: >> >> > if (!NodeKind.isSame(Other.NodeKind)) >> >> > return NodeKind < Other.NodeKind; >> >> > >> >> > + if (ASTNodeKind::getFromNodeKind<QualType>().isSame(NodeKind)) >> >> > + return getUnchecked<QualType>().getAsOpaquePtr() == >> >> > + Other.getUnchecked<QualType>().getAsOpaquePtr(); >> >> > + >> >> >> >> This only tests for equality, not comparison, which means operator< no >> >> longer has a strict weak ordering. Also, there are no tests for this >> >> change. >> >> >> >> ~Aaron >> >> >> > r255929 changes the operator to '<', which should fix this. I don't >> > have a >> > test case since the assertion was triggered in a recently checked in >> > Clang >> > Tidy test that only fails in a special configuration that I couldn't >> > reproduce in a clean change. >> >> Thank you, that commit looks much better! Couldn't we add a test in >> ASTTypeTraitsTests.cpp as a unit test, or was the assert specific to >> that particular special configuration? >> >> ~Aaron >> >> > >> >> > if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(NodeKind)) { >> >> > auto TLA = getUnchecked<TypeLoc>(); >> >> > auto TLB = Other.getUnchecked<TypeLoc>(); >> >> > >> >> > >> >> > _______________________________________________ >> >> > cfe-commits mailing list >> >> > cfe-commits@lists.llvm.org >> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >> > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits