sw/source/core/doc/docbm.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
New commits: commit 94be87df833772dcaf7e96659af78b5fa355c966 Author: Stephan Bergmann <sberg...@redhat.com> AuthorDate: Fri Jul 1 09:16:47 2022 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Fri Jul 1 13:59:36 2022 +0200 Use stable_sort for m_vBookmarks Using a randomizing debug-mode libc++ (`export LIBCPP_DEBUG=-D_LIBCPP_DEBUG=1` in config_host.mk), where (among other things) the elements of a range passed to std::sort are randomly shuffled before being sorted, thus simulating a std::sort that actually behaves non-deterministically compared to std::stable_sort: CppunitTest_sw_uiwriter5 consistently failed for me with > uiwriter5.cxx:255:Assertion > Test name: testTdf140982::TestBody > assertion failed > - Expression: aType != "AnnotationEnd" || !bAnnotationStart when _LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY_SEED is 140384686472728. Dumping the contents of xRunEnum in testTdf140982 (sw/qa/extras/uiwriter/uiwriter5.cxx), that randomized run has > Text "Lorem " > Redline IsStart=true > Annotation > Text "ipsum" > Redline IsStart=false > Redline IsStart=true > Annotation > AnnotationEnd <= the CPPUNIT_ASSERT failed here > Redline IsStart=false > Redline IsStart=true > Text " " > Annotation > Text "dolor" > Redline IsStart=false > Redline IsStart=true > AnnotationEnd > Redline IsStart=false > Redline IsStart=true > Text " " > Annotation > Text "sit" > Redline IsStart=false > Redline IsStart=true > AnnotationEnd > Redline IsStart=false > Text " amet" > Redline IsStart=true > AnnotationEnd > Redline IsStart=false > Text "..." whereas a stable_sort run would have > Text "Lorem " > RedLine IsStart=true > Annotation > Annotation > Text "ipsum" > RedLine IsStart=false > RedLine IsStart=true > AnnotationEnd > RedLine IsStart=false > RedLine IsStart=true > Text " " > Annotation > Text "dolor" > RedLine IsStart=false > RedLine IsStart=true > AnnotationEnd > RedLine IsStart=false > RedLine IsStart=true > Text " " > Annotation > Text "sit" > RedLine IsStart=false > RedLine IsStart=true > AnnotationEnd > RedLine IsStart=false > Text " amet" > RedLine IsStart=true > AnnotationEnd > RedLine IsStart=false > Text "..." which suspiciously has the Annotation tokens at different positions relative to the Text tokens, so something looks indeed broken. And when dumping (via ToString()) the content of m_vBookmarks in MarkManager::sortSubsetMarks (sw/source/core/doc/docbm.cxx; which is apparently called multiple times during that test) before and after sorting, there are often cases where multiple elements have the same node and index, so are placed into the same equivalence class by lcl_MarkOrderingByStart (assuming there are no sw::mark::CrossRefBookmark instances involved in this test, for which lcl_MarkOrderingByStart has special rules). I'm not exactly sure whether this use of stable_sort is indeed the right fix or just a workaround, and the real issue is that the equivalence classes of lcl_MarkOrderingByStart are too broad, or something entirely different yet. I'm also not sure whether similar uses of sort in MarkManager::sortSubsetMarks and MarkManager::sortMarks would also need to be changed to stable_sort. (At least, I didn't find any breakage with my randomizing debug-mode libc++ and `make check` that would point to one of those other non-stable sort calls.) Change-Id: I51040e43b906bd3f18219d3bd0d28e1ccee89897 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136717 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx index 1b283437ab6f..41d7a6dfb080 100644 --- a/sw/source/core/doc/docbm.cxx +++ b/sw/source/core/doc/docbm.cxx @@ -1738,7 +1738,7 @@ namespace sw::mark void MarkManager::sortSubsetMarks() { - sort(m_vBookmarks.begin(), m_vBookmarks.end(), &lcl_MarkOrderingByStart); + stable_sort(m_vBookmarks.begin(), m_vBookmarks.end(), &lcl_MarkOrderingByStart); sort(m_vFieldmarks.begin(), m_vFieldmarks.end(), &lcl_MarkOrderingByStart); sort(m_vAnnotationMarks.begin(), m_vAnnotationMarks.end(), &lcl_MarkOrderingByStart); }