Analitza's use of C99 variable-length arrays

[Percy Camilo Triveño Aucahuasi]

On Thu, Jul 3, 2014 at 9:04 PM, Nicolás Alvarez <nicolas.alvarez at gmail.com>
wrote:

> 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
>


I checked the tests and everything is fine (in this case analitzatest and
commandstest pass)
Next time you could do the same, you can run the appropriated test to see
if something is wrong with your fix ;)

Thanks again,
Percy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140703/0974e03c/attachment-0001.html>