[Libreoffice-commits] core.git: sw/source

[Stephan Bergmann (via logerrit)]

 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);
     }