Analitza's use of C99 variable-length arrays

[Nicolás Alvarez]

2014-07-03 23:00 GMT-03:00 Percy Camilo Triveño Aucahuasi
<percy.camilo.ta at gmail.com>:
> On Tue, Jul 1, 2014 at 5:39 AM, Aleix Pol <aleixpol at kde.org> wrote:
>>
>> On Tue, Jul 1, 2014 at 5:03 AM, Nicolás Alvarez
>> <nicolas.alvarez at gmail.com> wrote:
>>>
>>> Hi peeps,
>>>
>>> In analitza/commands/blockmatrixcommands.cpp, there is a variable-length
>>> array:
>>>
>>> const int firstVectorSize = firstVector->size();
>>> int blockpattern[firstVectorSize];
>>>
>>> This is a new feature in C99. The GCC manual says: "Variable-length
>>> automatic arrays are allowed in ISO C99, and as an extension GCC
>>> accepts them in C90 mode and in C++."
>>>
>>> MSVC doesn't support VLAs, at least not in C++ mode. And given the
>>> above quote, it seems it's non-portable to use it in C++ anyway, so
>>> (this time :P) we can't blame it on MSVC not being standards-compliant
>>> or something.
>>>
>>> I replaced it with this to make it compile:
>>> std::unique_ptr<int[]> blockpattern(new int[firstVectorSize]);
>>>
>>> But I don't know if this is acceptable (can I use C++11 in analitza?).
>>> ##c++ told me to just use
>>> std::vector<int> blockpattern(firstVectorSize, 0);
>>>
>>> which is a reasonable option, but it feels weird to use a std::vector
>>> for something that won't be resized.
>>>
>>> I welcome thoughts from someone who actually knows the Analitza code :)
>>>
>>
>> Hi,
>> Wouldn't that fix the issue?
>>
>> Aleix
>>
>> diff --git analitza/commands/blockmatrixcommands.cpp
>> analitza/commands/blockmatrixcommands.cpp
>> index 6ff20ef..568fbde 100644
>> --- analitza/commands/blockmatrixcommands.cpp
>> +++ analitza/commands/blockmatrixcommands.cpp
>> @@ -62,7 +62,7 @@ Expression BlockMatrixCommand::operator()(const QList<
>> Analitza::Expression >& a
>>   bool isCorrect = true; // this flag tells if is ok to build the block
>> matrix
>>   int nrows = 0;
>>   int ncols = 0;
>> - int blockpattern[firstVectorSize]; // if vectors(matrixrow) this tells
>> the row(column) pattern
>> + std::vector<int> blockpattern(firstVectorSize, 0); // if
>> vectors(matrixrow) this tells the row(column) pattern
>>
>>   const int blocklength = isVector? firstBlock->columnCount() :
>> firstBlock->rowCount();
>>
>>
>>
>
> Hi Nicolás,
>
> Please confirm us if the fix provided by Aleix is enough, I remember I did
> some test with that approach (using std::vector) and it works ok ... even
> more, I'm a bit surprised I left the code with a raw C99 array instead of
> std::vector ... :p

The fix using std::vector compiles in MSVC and has been already
committed. Whether it *works* or not (ie. gives the same result)... I
leave that to you to decide, since I'm not familiar with Analitza and
don't know what could possibly break if this particular function
breaks :)

-- 
Nicolás