[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-edu-devel
Subject: Re: Analitza's use of C99 variable-length arrays
From: Percy_Camilo_Triveño_Aucahuasi <percy.camilo.ta () gmail ! com>
Date: 2014-07-04 2:20:46
Message-ID: CAH5PjXoqPRC-Wx_yRoRfBxh_5LUvmsAU=8mMOi0HDaNd=73pFg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Thu, Jul 3, 2014 at 9:04 PM, Nicolás Alvarez <nicolas.alvarez@gmail.com>
wrote:
> 2014-07-03 23:00 GMT-03:00 Percy Camilo Triveño Aucahuasi
> <percy.camilo.ta@gmail.com>:
> > On Tue, Jul 1, 2014 at 5:39 AM, Aleix Pol <aleixpol@kde.org> wrote:
> >>
> >> On Tue, Jul 1, 2014 at 5:03 AM, Nicolás Alvarez
> >> <nicolas.alvarez@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
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 3, 2014 \
at 9:04 PM, Nicolás Alvarez <span dir="ltr"><<a \
href="mailto:nicolas.alvarez@gmail.com" \
target="_blank">nicolas.alvarez@gmail.com</a>></span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">2014-07-03 \
23:00 GMT-03:00 Percy Camilo Triveño Aucahuasi<br>
<<a href="mailto:percy.camilo.ta@gmail.com">percy.camilo.ta@gmail.com</a>>:<br>
<div><div class="h5">> On Tue, Jul 1, 2014 at 5:39 AM, Aleix Pol <<a \
href="mailto:aleixpol@kde.org">aleixpol@kde.org</a>> wrote:<br> >><br>
>> On Tue, Jul 1, 2014 at 5:03 AM, Nicolás Alvarez<br>
>> <<a href="mailto:nicolas.alvarez@gmail.com">nicolas.alvarez@gmail.com</a>> \
wrote:<br> >>><br>
>>> Hi peeps,<br>
>>><br>
>>> In analitza/commands/blockmatrixcommands.cpp, there is a \
variable-length<br> >>> array:<br>
>>><br>
>>> const int firstVectorSize = firstVector->size();<br>
>>> int blockpattern[firstVectorSize];<br>
>>><br>
>>> This is a new feature in C99. The GCC manual says: \
"Variable-length<br> >>> automatic arrays are allowed in ISO C99, and \
as an extension GCC<br> >>> accepts them in C90 mode and in C++."<br>
>>><br>
>>> MSVC doesn't support VLAs, at least not in C++ mode. And given \
the<br> >>> above quote, it seems it's non-portable to use it in C++ \
anyway, so<br> >>> (this time :P) we can't blame it on MSVC not being \
standards-compliant<br> >>> or something.<br>
>>><br>
>>> I replaced it with this to make it compile:<br>
>>> std::unique_ptr<int[]> blockpattern(new int[firstVectorSize]);<br>
>>><br>
>>> But I don't know if this is acceptable (can I use C++11 in \
analitza?).<br> >>> ##c++ told me to just use<br>
>>> std::vector<int> blockpattern(firstVectorSize, 0);<br>
>>><br>
>>> which is a reasonable option, but it feels weird to use a \
std::vector<br> >>> for something that won't be resized.<br>
>>><br>
>>> I welcome thoughts from someone who actually knows the Analitza code \
:)<br> >>><br>
>><br>
>> Hi,<br>
>> Wouldn't that fix the issue?<br>
>><br>
>> Aleix<br>
>><br>
>> diff --git analitza/commands/blockmatrixcommands.cpp<br>
>> analitza/commands/blockmatrixcommands.cpp<br>
>> index 6ff20ef..568fbde 100644<br>
>> --- analitza/commands/blockmatrixcommands.cpp<br>
>> +++ analitza/commands/blockmatrixcommands.cpp<br>
>> @@ -62,7 +62,7 @@ Expression BlockMatrixCommand::operator()(const \
QList<<br> >> Analitza::Expression >& a<br>
>> bool isCorrect = true; // this flag tells if is ok to build the block<br>
>> matrix<br>
>> int nrows = 0;<br>
>> int ncols = 0;<br>
>> - int blockpattern[firstVectorSize]; // if vectors(matrixrow) this tells<br>
>> the row(column) pattern<br>
>> + std::vector<int> blockpattern(firstVectorSize, 0); // if<br>
>> vectors(matrixrow) this tells the row(column) pattern<br>
>><br>
>> const int blocklength = isVector? firstBlock->columnCount() :<br>
>> firstBlock->rowCount();<br>
>><br>
>><br>
>><br>
><br>
> Hi Nicolás,<br>
><br>
> Please confirm us if the fix provided by Aleix is enough, I remember I did<br>
> some test with that approach (using std::vector) and it works ok ... even<br>
> more, I'm a bit surprised I left the code with a raw C99 array instead \
of<br> > std::vector ... :p<br>
<br>
</div></div>The fix using std::vector compiles in MSVC and has been already<br>
committed. Whether it *works* or not (ie. gives the same result)... I<br>
leave that to you to decide, since I'm not familiar with Analitza and<br>
don't know what could possibly break if this particular function<br>
breaks :)<br>
<div class=""><div class="h5"><br>
--<br>
Nicolás<br></div></div></blockquote><div><br></div><div><br></div><div>I checked the \
tests and everything is fine (in this case analitzatest and commandstest \
pass)</div><div>Next time you could do the same, you can run the appropriated test to \
see if something is wrong with your fix ;)</div> <div><br></div><div>Thanks \
again,</div><div>Percy</div><div><br></div></div></div></div>
_______________________________________________
kde-edu mailing list
kde-edu@mail.kde.org
https://mail.kde.org/mailman/listinfo/kde-edu
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic