BUG: bitSet &= operation does not mask out non-overlapping (#2456)

- the values from non-overlapping blocks were simply ignored,
  which meant that ('111111111111' & '111111') would not mask out
  the unset values at all.

- similar oddities in other operations (|=, ^= etc)
  where the original implementation tried hard to avoid touching the
  sizing at all, but now better resolved as follows:

  - '|=' : Set may grow to accommodate new 'on' bits.
  - '^=' : Set may grow to accommodate new 'on' bits.
  - '-=' : Never changes the original set size.
  - '&=' : Never changes the original set size.
           Non-overlapping elements are considered 'off'.

  These definitions are consistent with HashSet behaviour
  and also ensures that (a & b) == (b & a)

ENH: improve short-circuiting within bitSet ops

- in a few places can optimise by checking for none() instead of
  empty() and avoid unnecessary block operations.

ENH: added bitSet::resize_last() method

- as the name says: resizes to the last bit set.
  A friendlier way of writing `resize(find_last()+1)`

applications/test/bitSet2/Test-bitSet2.C

@@ -344,12 +344,35 @@ int main(int argc, char *argv[])
 
     Info<< "\noperator|=\n";
     {
+        bitSet a; a.set(labelRange(15, 8));
+        bitSet b; b.set(labelRange(30, 8));
+
+        report(a, false);
+        report(b, false);
+        report((a |= b), true);
+    }
+    {
+        Info<< " other test\n";
+
         bitSet list3 = list1;
         report((list3 |= list2), true);
     }
 
     Info<< "\noperator&=\n";
     {
+        bitSet a; a.set(labelRange(30, 8));
+        bitSet b; b.set(labelRange(0, 8));
+
+        report(a, false);
+        report(b, false);
+        Info<< "  (a & b)\n";
+        report((a & b), true);
+        Info<< "  (b & a)\n";
+        report((b & a), true);
+    }
+    {
+        Info<< " other test\n";
+
         bitSet list3 = list1;
         report((list3 &= list2), true);
     }

src/OpenFOAM/containers/Bits/PackedList/PackedList.H

@@ -344,7 +344,7 @@ public:
         //- a minimum position, below which trimming will not occur.
         //
         //  \return true if trimming changed the size.
-        inline bool trim(label minpos=-1);
+        inline bool trim(label minpos = -1);
 
         //- Clear all bits but do not adjust the addressable size.
         //  \note Method name compatibility with boost::dynamic_bitset

src/OpenFOAM/containers/Bits/bitSet/bitSet.C

@@ -43,7 +43,7 @@ Foam::bitSet& Foam::bitSet::minusEq(const bitSet& other)
 {
     if (&other == this)
     {
-        // Self '-=' : results in clearing all bits
+        // Self '-=' : clears all bits
         if (debug & 2)
         {
             InfoInFunction
@@ -53,12 +53,12 @@ Foam::bitSet& Foam::bitSet::minusEq(const bitSet& other)
         reset();
         return *this;
     }
-    else if (empty() || other.empty())
+    else if (none() || other.none())
     {
+        // no-op: nothing can change
         return *this;
     }
 
-
     // The operation (on overlapping blocks)
     {
         const label nblocks = num_blocks(std::min(size(), other.size()));
@@ -88,21 +88,32 @@ Foam::bitSet& Foam::bitSet::andEq(const bitSet& other)
 
         return *this;
     }
-    else if (empty())
+    else if (none())
     {
-        // empty set : no-op (no overlap possible)
+        // no-op: nothing is set - no intersection possible
         return *this;
     }
-    else if (other.empty())
+    else if (other.none())
     {
-        reset();  // Other is empty - no overlap possible
+        // no-op: other has nothing set - no intersection possible
+        reset();
         return *this;
     }
 
 
+    const label origSize(size());
+    const label otherSize(other.size());
+
+    if (origSize > otherSize)
+    {
+        // Clear bits (and blocks) that do not overlap at all
+        resize(otherSize);
+        resize(origSize);
+    }
+
     // The operation (on overlapping blocks)
     {
-        const label nblocks = num_blocks(std::min(size(), other.size()));
+        const label nblocks = num_blocks(std::min(origSize, otherSize));
         const auto& rhs = other.blocks_;
 
         for (label blocki = 0; blocki < nblocks; ++blocki)
@@ -115,7 +126,7 @@ Foam::bitSet& Foam::bitSet::andEq(const bitSet& other)
 }
 
 
-Foam::bitSet& Foam::bitSet::orEq(const bitSet& other, const bool strict)
+Foam::bitSet& Foam::bitSet::orEq(const bitSet& other)
 {
     if (&other == this)
     {
@@ -129,52 +140,21 @@ Foam::bitSet& Foam::bitSet::orEq(const bitSet& other, const bool strict)
 
         return *this;
     }
-    else if (other.empty())
+    else if (other.none())
     {
-        if ((debug & 2) && !empty())
+        // no-op: nothing can change
-        {
-            // OK if both are empty
-            InfoInFunction
-                << "Perform |= using empty operand: ignore" << nl;
-        }
-
-        // No (normal) overlap: no-op
         return *this;
     }
-    else if (empty())
+
+
+    // Largest new bit that could be introduced
+    const label otherMax(other.find_last());
+
+    if (otherMax >= size())
     {
-        if (debug & 2)
+        // Extend to accommodate bits from 'other'
-        {
+        resize(otherMax+1);
-            InfoInFunction
-                << "Perform |= on empty bitSet" << nl;
-        }
-
-        if (strict)
-        {
-            // No (normal) overlap: no-op
-            return *this;
-        }
     }
-    else if ((debug & 2) && (size() != other.size()))
-    {
-        InfoInFunction
-            << "Perform |= on dissimilar sized bitSets: "
-            << size()  << " vs. " << other.size() << nl;
-    }
-
-    label minpos = -1; // Min trim point
-
-    if ((size() < other.size()) && !strict)
-    {
-        // The size (B > A) and we are non-strict (greedy), which means we may
-        // acquire additional bits from B. However, we would like to avoid
-        // spurious changes in the size of A (ie, B is longer but the extra
-        // bits are unset and thus don't affect the logical result).
-
-        minpos = size();
-        resize(other.size());   // Blocks now overlap
-    }
-
 
     // The operation (on overlapping blocks)
     {
@@ -187,26 +167,15 @@ Foam::bitSet& Foam::bitSet::orEq(const bitSet& other, const bool strict)
         }
     }
 
-
-    // Cleanup - minpos >= 0 means we need to check/adjust the trim point
-    if (minpos >= 0)
-    {
-        trim(minpos); // Adjust the trim point (size)
-    }
-    else
-    {
-        clear_trailing_bits();
-    }
-
     return *this;
 }
 
 
-Foam::bitSet& Foam::bitSet::xorEq(const bitSet& other, const bool strict)
+Foam::bitSet& Foam::bitSet::xorEq(const bitSet& other)
 {
     if (&other == this)
     {
-        // Self '^=' : results in clearing all bits
+        // Self '^=' : clears all bits
 
         if (debug & 2)
         {
@@ -217,47 +186,21 @@ Foam::bitSet& Foam::bitSet::xorEq(const bitSet& other, const bool strict)
         reset();
         return *this;
     }
-    else if (other.empty())
+    else if (other.none())
     {
-        if ((debug & 2) && !empty())
+        // no-op: nothing can change
-        {
-            // OK if both are empty
-            InfoInFunction
-                << "Perform ^= using empty operand: ignore" << nl;
-        }
-
-        // No (normal) overlap: no-op
         return *this;
     }
-    else if (empty())
+
+
+    // Largest new bit that could be introduced
+    const label otherMax(other.find_last());
+
+    if (otherMax >= size())
     {
-        if (debug & 2)
+        // Extend to accommodate bits from 'other'
-        {
+        resize(otherMax+1);
-            InfoInFunction
-                << "Perform ^= on empty bitSet" << nl;
-        }
-
-        if (strict)
-        {
-            // No (normal) overlap: no-op
-            return *this;
-        }
     }
-    else if ((debug & 2) && (size() != other.size()))
-    {
-        InfoInFunction
-            << "Perform ^= on dissimilar sized bitSets: "
-            << size()  << " vs. " << other.size() << nl;
-    }
-
-    label minpos = -1; // Min trim point
-
-    if ((size() < other.size()) && !strict)
-    {
-        minpos = size();        // This logic is explained in the orEq() method
-        resize(other.size());   // Blocks now overlap
-    }
-
 
     // The operation (on overlapping blocks)
     {
@@ -270,17 +213,6 @@ Foam::bitSet& Foam::bitSet::xorEq(const bitSet& other, const bool strict)
         }
     }
 
-
-    // Cleanup - minpos >= 0 means we need to check/adjust the trim point
-    if (minpos >= 0)
-    {
-        trim(minpos); // Adjust the trim point (size)
-    }
-    else
-    {
-        clear_trailing_bits();
-    }
-
     return *this;
 }
 

src/OpenFOAM/containers/Bits/bitSet/bitSet.H

@@ -83,7 +83,8 @@ protected:
         //      A = (A & ~B)
         //  \endcode
         //  A and B can have different sizes.
-        //  Does not change the original set size.
+        //  Never changes the original set size.
+        //  Non-overlapping parts are considered \em off.
         bitSet& minusEq(const bitSet& other);
 
         //- The set logical AND
@@ -91,8 +92,9 @@ protected:
         //      A = (A & B)
         //
         //  \endcode
-        //  A and B can have different sizes..
+        //  A and B can have different sizes.
-        //  Does not change the original set size.
+        //  Never changes the original set size.
+        //  Non-overlapping parts are considered \em off.
         bitSet& andEq(const bitSet& other);
 
         //- The set logical OR
@@ -101,18 +103,17 @@ protected:
         //  \endcode
         //  A and B can have different sizes
         //
-        //  \note
+        //  The size grows to accommodate new \em on bits.
-        //      The default (strict=true) ignores additional length from B,
+        bitSet& orEq(const bitSet& other);
-        //      whereas (strict=false) permits the set to automatically grow
-        //      to accommodate additional elements arising from B.
-        bitSet& orEq(const bitSet& other, const bool strict=true);
 
         //- The set logical XOR
         //  \code
         //      A = (A ^ B)
         //  \endcode
-        //  A and B can have different sizes. Sizing behaviour as per orEq.
+        //  A and B can have different sizes.
-        bitSet& xorEq(const bitSet& other, const bool strict=true);
+        //
+        //  The size grows to accommodate new \em on bits.
+        bitSet& xorEq(const bitSet& other);
 
 
 public:
@@ -123,6 +124,7 @@ public:
         class const_iterator;
         typedef unsigned int const_reference;
 
+
     // Static Member Functions
 
         //- Return a null bitSet reference
@@ -363,10 +365,14 @@ public:
         //- Invert all bits in the addressable region
         inline void flip();
 
-        //- Invert bits at the specified position.
+        //- Invert bit at the specified position.
         //  A no-op if the position is out-of-range
         inline void flip(const label pos);
 
+        //- Resize to include the last \em on bit only.
+        //  Functionally identical to resize(find_last()+1)
+        inline void resize_last();
+
         //- Swap contents
         inline void swap(bitSet& bitset);
 
@@ -573,23 +579,24 @@ public:
         inline bitSet& operator=(const bool val);
 
         //- Bitwise-AND all the bits in other with the bits in this bitset.
-        //  The operands may have dissimilar sizes without affecting the size
+        //  The operands may have dissimilar sizes,
-        //  of the set.
+        //  never changes the original set size.
+        //  Non-overlapping elements are considered \em off.
         inline bitSet& operator&=(const bitSet& other);
 
         //- Bitwise-OR operator - similar to the set() method.
-        //  The operands may have dissimilar sizes without affecting the size
+        //  The operands may have dissimilar sizes,
-        //  of the set.
+        //  the set grows to accommodate new \em on bits.
         inline bitSet& operator|=(const bitSet& other);
 
         //- Bitwise-XOR operator - retains unique entries.
-        //  The operands may have dissimilar sizes without affecting the size
+        //  The operands may have dissimilar sizes,
-        //  of the set.
+        //  the set grows to accommodate new \em on bits.
         inline bitSet& operator^=(const bitSet& other);
 
         //- Remove entries from this list - identical to the unset() method.
-        //  The operands may have dissimilar sizes without affecting the size
+        //  The operands may have dissimilar sizes,
-        //  of the set.
+        //  never changes the original set size.
         inline bitSet& operator-=(const bitSet& other);
 
 

src/OpenFOAM/containers/Bits/bitSet/bitSetI.H

@@ -5,7 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Copyright (C) 2018-2020 OpenCFD Ltd.
+    Copyright (C) 2018-2022 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -316,6 +316,7 @@ inline Foam::label Foam::bitSet::find_first() const
     // Process block-wise, detecting any '1' bits
 
     const label nblocks = num_blocks(size());
+
     for (label blocki = 0; blocki < nblocks; ++blocki)
     {
         label pos = (blocki * elem_per_block);
@@ -535,6 +536,21 @@ inline Foam::labelList Foam::bitSet::sortedToc() const
 }
 
 
+inline void Foam::bitSet::resize_last()
+{
+    const label pos = find_last();
+
+    if (pos >= 0)
+    {
+        resize(pos+1);
+    }
+    else
+    {
+        clear();
+    }
+}
+
+
 inline void Foam::bitSet::swap(bitSet& bitset)
 {
     PackedList<1>::swap(bitset);
@@ -573,7 +589,7 @@ inline void Foam::bitSet::fill(const bool val)
 
 inline void Foam::bitSet::set(const bitSet& bitset)
 {
-    orEq(bitset, false);  // Non-strict: Lets the set size grow.
+    orEq(bitset);
 }
 
 
@@ -754,28 +770,40 @@ inline Foam::bitSet Foam::operator~(const bitSet& bitset)
 inline Foam::bitSet Foam::operator&(const bitSet& a, const bitSet& b)
 {
     bitSet result(a);
-    return (result &= b);
+    result &= b;
+
+    result.resize_last();
+    return result;
 }
 
 
 inline Foam::bitSet Foam::operator|(const bitSet& a, const bitSet& b)
 {
     bitSet result(a);
-    return (result |= b);
+    result |= b;
+
+    result.resize_last();
+    return result;
 }
 
 
 inline Foam::bitSet Foam::operator^(const bitSet& a, const bitSet& b)
 {
     bitSet result(a);
-    return (result ^= b);
+    result ^= b;
+
+    result.resize_last();
+    return result;
 }
 
 
 inline Foam::bitSet Foam::operator-(const bitSet& a, const bitSet& b)
 {
     bitSet result(a);
-    return (result -= b);
+    result |= b;
+
+    result.resize_last();
+    return result;
 }