HPCC-16812 Fix multiple problems with bitfields

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hql/hqlfold.cpp

@@ -2192,6 +2192,11 @@ IHqlExpression * applyBinaryFold(IHqlExpression * expr, binaryFoldFunc folder)
     {
         IValue * res = folder(leftValue, rightValue);
         assertex(res);
+#if 0
+        //A useful consistency test, but not always true for no_concat, so commented out for the moment
+        if (!isUnknownSize(expr->queryType()))
+            assertex(res->queryType() == expr->queryType());
+#endif
         return createConstant(res);
     }
 

ecl/hql/hqllex.l

@@ -1333,7 +1333,8 @@ BITFIELD{digit}+    {
                          size = 1;
                       }
 
-                      ITypeInfo * type = makeBitfieldType(size);
+                      Owned<ITypeInfo> int64 = makeIntType(8, false);
+                      ITypeInfo * type = makeBitfieldType(size, int64);
                       assert(type!=NULL);
 
                       returnToken.setType(type); 
@@ -1358,7 +1359,7 @@ BITFIELD{digit}+_{digit}+ {
                       case 1: case 2: case 4: case 8:
                          break;
                       default:
-                         lexer->reportError(returnToken, ERR_ILLSIZE_BITFIELD, "Invalid size for BITFIELD type: valid size 1..64");
+                         lexer->reportError(returnToken, ERR_ILLSIZE_BITFIELD, "Invalid size for BITFIELD base type: valid size 1,2,4,8");
                          baseSize = 8;
                       }
                       if (size > baseSize*8)

ecl/hql/hqlutil.cpp

@@ -4794,9 +4794,10 @@ bool BitfieldPacker::checkSpaceAvailable(unsigned & thisBitOffset, unsigned & th
 {
     bool fitted = true;
     thisBits = type->getBitSize();
-    if (thisBits > bitsRemaining)
+    ITypeInfo * storeType = type->queryChildType();
+    if ((thisBits > bitsRemaining) || !activeType || (storeType != activeType))
     {
-        ITypeInfo * storeType = type->queryChildType();
+        activeType = storeType;
         unsigned unitSize = storeType->getSize();
         bitsRemaining = unitSize * 8;
         nextBitOffset = 0;

ecl/hql/hqlutil.hpp

@@ -453,11 +453,13 @@ public:
 
     void reset()
     {
+        activeType = nullptr;
         bitsRemaining = 0;
         nextBitOffset = 0;
     }
 
 public:
+    ITypeInfo * activeType = nullptr;
     unsigned bitsRemaining;
     unsigned nextBitOffset;
 };

ecl/hqlcpp/hqlhtcpp.cpp

@@ -3888,7 +3888,7 @@ unsigned HqlCppTranslator::buildRtlType(StringBuffer & instanceName, ITypeInfo *
     case type_bitfield:
         {
         className.append("RtlBitfieldTypeInfo");
-        unsigned size = type->getSize();
+        unsigned size = type->queryChildType()->getSize();
         unsigned bitsize = type->getBitSize();
         unsigned offset = (unsigned)getIntValue(queryAttributeChild(type, bitfieldOffsetAtom, 0),-1);
         bool isLastBitfield = (queryAttribute(type, isLastBitfieldAtom) != NULL);

ecl/hqlcpp/hqltcppc.cpp

@@ -2305,8 +2305,7 @@ void CBitfieldContainerInfo::gatherSize(SizeStruct & size)
 
 IHqlExpression * CBitfieldContainerInfo::getRelativeSelf()
 {
-    //ifblocks inside bitfield containers?? Not supported.... should create a new container, unless desparate.
-    UNIMPLEMENTED;
+    return container->getRelativeSelf();
 }
 
 
@@ -2376,7 +2375,10 @@ void CBitfieldInfo::setColumn(HqlCppTranslator & translator, BuildCtx & ctx, IRe
 
     unsigned bitSize = columnType->getBitSize();
     unsigned __int64 mask = (((unsigned __int64)1)<<bitSize)-1;
-    unsigned __int64 shiftMask = (((unsigned __int64)1)<<(bitSize+bitOffset)) - (((unsigned __int64)1)<<bitOffset);
+    unsigned __int64 shiftMask = 0;
+    if (bitSize + bitOffset < sizeof(__int64) * 8)
+        shiftMask = (((unsigned __int64)1)<<(bitSize+bitOffset));
+    shiftMask -= (((unsigned __int64)1)<<bitOffset);
     IValue * oldMaskValue = storageType->castFrom(false, (__int64)shiftMask);
     IHqlExpression * oldMask = createConstant(oldMaskValue);
     IHqlExpression * transColumn = createTranslatedOwned(columnRef);
@@ -2384,7 +2386,7 @@ void CBitfieldInfo::setColumn(HqlCppTranslator & translator, BuildCtx & ctx, IRe
 
     IValue * newMaskValue = storageType->castFrom(false, (__int64)mask);
     IHqlExpression * newMask = createConstant(newMaskValue);
-    OwnedHqlExpr newValue = createValue(no_band, LINK(storageType), LINK(value), newMask);
+    OwnedHqlExpr newValue = createValue(no_band, LINK(storageType), ensureExprType(value, storageType), newMask);
     if (bitOffset > 0)
         newValue.setown(createValue(no_lshift, LINK(storageType), newValue.getClear(), getSizetConstant(bitOffset)));
     if (newValue->isConstant())
@@ -3029,7 +3031,8 @@ CMemberInfo * ColumnToOffsetMap::createColumn(CContainerInfo * container, IHqlEx
                 if (prior)
                     prior->noteLastBitfield();
                 ITypeInfo * storeType = type->queryChildType();
-                OwnedHqlExpr value = createValue(no_field, LINK(storeType));
+                //Use createUniqueId() to ensure that bitfield containers of the same type can be distinguished.
+                OwnedHqlExpr value = createValue(no_field, LINK(storeType), createUniqueId());
                 bitContainer = new CBitfieldContainerInfo(container, prior, value);
                 bitContainer->setOffset(!fixedSizeRecord);
             }

ecl/regress/bitfield3.ecl

@@ -17,8 +17,8 @@
 
 in1rec := RECORD
     UNSIGNED1 id;
-    BITFIELD4 age1;
-    BITFIELD4 age2;
+    BITFIELD4_1 age1;
+    BITFIELD4_1 age2;
 END;
 
 in1 := DATASET([{1,10,12},

ecl/regress/default1.ecl

@@ -19,8 +19,8 @@ r1 := RECORD
     unsigned1   f1{default(0)};
     unsigned1   f2{default(255)};
     unsigned1   badf3{default(256)};
-    bitfield3   f4{default(7)};
-    bitfield3   badf5{default(8)};
+    bitfield3_1   f4{default(7)};
+    bitfield3_1   badf5{default(8)};
 END;
 
 output(dataset(row(transform(r1, self := []))));

rtl/eclrtl/rtlfield.cpp

@@ -1384,20 +1384,20 @@ __int64 RtlBitfieldTypeInfo::signedValue(const byte * self) const
     __int64 value = rtlReadInt(self, getBitfieldIntSize());
     unsigned shift = getBitfieldShift();
     unsigned numBits = getBitfieldNumBits();
-    value <<= (sizeof(value) - shift - numBits);
-    return value >> numBits;
-
+    unsigned bitsInValue = sizeof(value) * 8;
+    value <<= (bitsInValue - shift - numBits);
+    return value >> (bitsInValue - numBits);
 }
 
 
 unsigned __int64 RtlBitfieldTypeInfo::unsignedValue(const byte * self) const
 {
-    unsigned __int64 value = rtlReadInt(self, getBitfieldIntSize());
+    unsigned __int64 value = rtlReadUInt(self, getBitfieldIntSize());
     unsigned shift = getBitfieldShift();
     unsigned numBits = getBitfieldNumBits();
-    value <<= (sizeof(value) - shift - numBits);
-    return value >> numBits;
-
+    unsigned bitsInValue = sizeof(value) * 8;
+    value <<= (bitsInValue - shift - numBits);
+    return value >> (bitsInValue - numBits);
 }
 
 
@@ -1419,12 +1419,15 @@ size32_t RtlBitfieldTypeInfo::process(const byte * self, const byte * selfrow, c
 
 size32_t RtlBitfieldTypeInfo::toXML(const byte * self, const byte * selfrow, const RtlFieldInfo * field, IXmlWriter & target) const
 {
-    size32_t fieldsize = size(self, selfrow);
+    size32_t fieldsize = getBitfieldIntSize();
     if (isUnsigned())
         target.outputUInt(unsignedValue(self), fieldsize, queryScalarXPath(field));
     else
         target.outputInt(signedValue(self), fieldsize, queryScalarXPath(field));
-    return fieldsize;
+
+    if (fieldType & RFTMislastbitfield)
+        return fieldsize;
+    return 0;
 }
 
 //-------------------------------------------------------------------------------------------------------------------

testing/regress/ecl/xmlout2.ecl

@@ -49,12 +49,17 @@ phoneRecord         mobilePhone;
 contactRecord   contact;
 dataset(bookRec) books;
 set of string    colours;
+BITFIELD63      b1;
+BITFIELD1       b2;
+BITFIELD3_8     b3;
+BITFIELD12_8    b4;
+BITFIELD17_8    b5;
 string2         endmarker := '$$';
             END;
 
 namesTable := dataset([
-        {'Halliday','Gavin','09876',123456,true,'07967',838690, 'n/a','n/a',true,'gavin@edata.com',[{'To kill a mocking bird','Lee'},{'Zen and the art of motorcycle maintainence','Pirsig'}], ALL},
-        {'Halliday','Abigail','09876',654321,false,'','',false,[{'The cat in the hat','Suess'},{'Wolly the sheep',''}], ['Red','Yellow']}
+        {'Halliday','Gavin','09876',123456,true,'07967',838690, 'n/a','n/a',true,'gavin@edata.com',[{'To kill a mocking bird','Lee'},{'Zen and the art of motorcycle maintainence','Pirsig'}], ALL, 1234567,0,7,6,5},
+        {'Halliday','Abigail','09876',654321,false,'','',false,[{'The cat in the hat','Suess'},{'Wolly the sheep',''}], ['Red','Yellow'], 0x7fffffffffffffff,1,7,4095,0x1FFFF}
         ], personRecord);
 
 output(namesTable,,'REGRESS::TEMP::output2.xml',overwrite,xml(heading('','')));