Dubious use of brace-init for OpenFOAM lists

In various places there are these types of constructs:

const List<label>& count = {}

This is OK since it will create a zero-sized labelList. However, in general it would be preferable to write as one of these:

const List<label>& count = List<label>();
const List<label>& count = List<label>::null();

• For the first one, the null constructor (in OpenFOAM) is constexpr noexcept, so the compiler should just rip it out at compile time.
• For the second one, in OpenFOAM, the static null() method will cast back from a tuple of zeros (ie, nullptr, len = 0) and the compiler will skip over at execution time.

Neither of these are showstoppers. However, the following code really worries me:

adiosStreamPtr->transfer
(
   fde.id(),
   {nCmpts*nGlobalElems},
   {nCmpts*elemOffset},
   {nCmpts*nElems},
   reinterpret_cast<const scalar*>(fde.uList().cdata())
);

In OpenFOAM (probably not foam-extend), the List constructor accepts a std::initializer_list, which is what will be taken. So the array shape is now a labelList with size nCmpts*nGlobalElems of uninitialized values.

Could/should write as

labelList(1, nCmpts*nGlobalElems)

Since we've kicked out the construct from iterator pairs, there should be no ambiguities. However, can also be a bit more certain and use the size/dispatch tag:

labelList(Foam::one{}, nCmpts*nGlobalElems)

assigned to @gregorweiss

changed the description

Using List<label>() or List<label>::null() is a helpful hint to use the mentioned compile time optimizations.

Are you sure that the results of

List<label>({nCmpts*nGlobalElems});

and

List<label>(1, nCmpts*nGlobalElems);

are not the same? IMHO the former does not generate a labelList with size nCmpts*nGlobalElems of uninitialized values. The List constructor in OpenFOAM accepting the std::initializer_list delegates to this constructor

template<class T>
template<class InputIterator>
inline Foam::List<T>::List
(
    InputIterator begIter,
    InputIterator endIter,
    const label len
)
:
    UList<T>(nullptr, len)
{
    if (this->size_)
    {
        doAlloc();

        InputIterator iter = begIter;
        for (label i = 0; i < len; ++i)
        {
            this->operator[](i) = *iter;
            ++iter;
        }
    }
}

which fills the constructed List with the values from the std::initializer_list.

The whole idea of using std::initializer_list for implicit List construction roots from being able to call the transfer on multi-dimensional data:

adiosStreamPtr->transfer
(
   fde.id(),
   {nGlobalElems, nCmpts},
   {elemOffset, nCmpts},
   {nElems, nCmpts},
   fde.uList().cdata()
);

I think you are right. Maybe the problem I've seen was List<label>{a,b} being intended as List<label>(a,b) but being interpreted as List<label>({a, b}) - causing fun bugs.

Probably that's what has stayed in my memory.

closed