patchGNU Octave - Patches: patch #8464, Make lssa clang compliant (that...

 
 

patch #8464: Make lssa clang compliant (that is, make it correct C++).

Submitter:  None
Submitted:  Sun 18 May 2014 06:32:52 PM UTC
   
 
Category:  Forge : other Priority:  5 - Normal
Status:  Wont Do Privacy:  Public
Assigned to:  None Originator Email:  -email is unavailable-
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 07 Mar 2019 12:09:25 AM UTC, comment #2: 

I think this has already been fixed separately in the lssa repository and lssa version 0.1.3 (see bug #48425).

Unfortunately the patch tracker is not as well visited as the bug tracker.

Mike Miller <mtmiller>
Group Member
Mon 19 May 2014 05:40:48 PM UTC, 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.

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Sun 18 May 2014 06:32:52 PM UTC, original submission:  

If compiling the package lssa with clang, it quits with an error.  The problem is that in strict C++, the functions "real" and "image" in std::complex do not return lvalues.  gcc relaxes the rules and allows them to be lvalues, but clang doesn't.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #31401:  stephens-patch-for-fixing-octave.diff added by None (3KiB - text/x-patch)

 

Depends on the following items: None found

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-07 mtmiller CategoryNone Forge : other
        StatusIn Progress Wont Do
        Open/ClosedOpen Closed
    2016-07-06 lachlan Dependencies- patch #7925 is dependent
    2014-05-19 jordigh StatusNone In Progress
    2014-05-18 None Attached File- Added stephens-patch-for-fixing-octave.diff, #31401

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code