danlark added a comment. In D155809#4553727 <https://reviews.llvm.org/D155809#4553727>, @aaron.ballman wrote:
> In D155809#4553694 <https://reviews.llvm.org/D155809#4553694>, @danlark wrote: > >> In D155809#4553690 <https://reviews.llvm.org/D155809#4553690>, >> @aaron.ballman wrote: >> >>> In D155809#4551876 <https://reviews.llvm.org/D155809#4551876>, @danlark >>> wrote: >>> >>>> In D155809#4551721 <https://reviews.llvm.org/D155809#4551721>, @cor3ntin >>>> wrote: >>>> >>>>> I think the test we want is a set of classes and virtuaal fuc >>>>> >>>>> In D155809#4551686 <https://reviews.llvm.org/D155809#4551686>, @philnik >>>>> wrote: >>>>> >>>>>> @aaron.ballman I think what @danlark means is that when building clang >>>>>> against a libc++ which has debug assertions enabled, the clang tests he >>>>>> mentioned result in an assertion firing within libc++. i.e. the libc++ >>>>>> debug mode catches the non-strict weak ordering that it gets from the >>>>>> clang code. That's why he can't just add a test that fires the >>>>>> assertion. Currently, there are no bots that build clang against a >>>>>> libc++ with debug assertions enabled. >>>>> >>>>> What we are asking for is a standalone minimal test that doesn't depend >>>>> on libc++ at all, living in `clang/test/` >>>> >>>> That's impossible. No test will fail because no implementation of >>>> std::stable_sort calls `comp(a, a)`. Only libcxx calls it in debug mode. >>>> But by C++ standard rules, `comp(a, a)` is allowed to be called. >>> >>> It's not impossible. :-) Can you share a link to which libc++ test case is >>> failing in debug mode? We can work backwards from there to build up a test >>> case. >> >> The issue is not in libc++, the assert fires if you build clang in the >> following way: >> >> 1. Build clang binary from source with the latest libc++ at head. >> 2. Build clang binary from source with -D_LIBCPP_ENABLE_DEBUG_MODE=1 >> -D_LIBCPP_ENABLE_HARDENED_MODE=1 (that's a libc++ define controlling >> standard library behavior which every file in the clang project uses, >> including this piece of code) >> 3. Run llvm-lit on clang tests >> 4. You will see 2 failures in >> llvm/llvm-project/clang/test/CodeGen:available-externally-hidden.cpp.test >> and >> llvm/llvm-project/clang/test/CodeGenCXX:cxx2a-three-way-comparison.cpp.test ( >> >> Without steps 1 and 2, there is no way to have assert on line 1569 >> <https://github.com/llvm/llvm-project/blob/2773098ee3187d5f9daca8938d57657dd89dd36f/clang/lib/AST/VTableBuilder.cpp#L1569>. >> No implementation of std::stable_sort in any major standard library can >> reach this line > > OH! The critical bit I was missing that you're building *Clang with libc++* > in order to see this issue, and the problem is that this particular call to > `std::stable_sort` has UB because of a violated precondition. Thanks for > sticking with the discussion until we had this clarity. > > The changes LGTM now that I understand the context better. This doesn't seem > to warrant a release note though, as users wouldn't see any differences. Thank you, I'll improve my communication so that we reach to an agreement faster. I don't have commit rights. Danila Kutenin. kutdan...@yandex.ru Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155809/new/ https://reviews.llvm.org/D155809 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
