octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #8464] Make lssa clang compliant (that is,


From: Jordi Gutiérrez Hermoso
Subject: [Octave-patch-tracker] [patch #8464] Make lssa clang compliant (that is, make it correct C++).
Date: Mon, 19 May 2014 17:40:48 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 Iceweasel/29.0.1

Update of patch #8464 (project octave):

                  Status:                    None => In Progress            

    _______________________________________________________

Follow-up Comment #1:

Your patch might be technically correct, but the commit message is a bit too
pithy.

How about instead something like


fastlcomplex.cc: don't treat imag and real as lvalues

Standard C++ doesn't specify that std::complex<T>::real() and
std::complex<T>::imag() can return lvalues, so they can't be assigned to. It
is a GNU C++ stdlib extension to return an lvalue, which other implementations
may not allow.

* fastlcomplex.cc (flscomplex): Add temporary fl


Your username in the patch is also rather nonstandard. Do you prefer that
username instead of your full name followed by an email address? Is this
username what you set in your ~/.hgrc?

You can further amend your patch by editing files and doing


## ci is an alias for commit
hg ci --user "Stephen Montgomery Etc" --amend


One further point: have you profiled this to ensure this doesn't result in a
slowdown? It calls a ctor and dtor on each iteration of a loop, doesn't it?
There might be a better approach.

I hope my review of your patch does not cause you any undue burden.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8464>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]