aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
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. 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
