bugGNU Octave - Bugs: bug #54237, 'abs' function template hack in...

 
 

bug #54237: 'abs' function template hack in pr-output.cc is not portable

Submitter:  Mike Miller <mtmiller>
Submitted:  Mon 02 Jul 2018 09:41:55 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Build Failure
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 10 Jul 2018 02:46:08 AM UTC, comment #7: 

I pushed a change on the stable branch for Octave 4.4.1 to fix the possible compiler warning about an unused function declaration

https://hg.savannah.gnu.org/hgweb/octave/rev/e8961d677661

I'm closing this as fixed for now, the ambiguous override error needs more information to be investigated.

 Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 09:29:44 PM UTC, comment #6: 

Ok, then I'll consider the failure to compile as a won't fix for now.

 Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 08:49:13 PM UTC, comment #5: 

Your patch fixed those problems so for now I'll stick to 4.92.

 Michele <michele31415>
Tue 03 Jul 2018 08:29:53 PM UTC, comment #4: 

cc Michele - maybe you can test a newer version of GCC 5 or 6 on your Solaris system to see whether this 'abs' resolution problem is solved?

 Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 07:47:36 PM UTC, comment #3: 

I am still bewildered as to how the compiler is attempting to match  the bare function name 'abs' against various 'std::abs' definitions.

The only thing I can think of is that the argument to 'abs' is resolved as a namespaced 'std::int32_t' for example, instead of a bare 'int32_t' type, which allows the compiler to resolve functions on it in the 'std' namespace. In the Octave source code itself, we don't use 'std::int32_t' anywhere, but maybe some difference in the Solaris system header files is making that equivalence.

 Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 06:34:20 PM UTC, comment #2: 

The updated attached patch fixes both of these problems for me. I think this is relatively simple and suitable for the stable branch.

  • Clang unused function warning is solved by declaring overrides for the eight intN_t and uintN_t types instead of the ten builtin int types.
  • Solaris ambiguous overload resolution error is solved by renaming the function from 'abs' to 'int_abs', seems stupid but without further information this may be necessary on some operating systems / compilers.


These problems can certainly be separated and applied individually. If we don't want to dumb our code down for the error on Solaris I would be fine with keeping the name 'abs' for now and waiting for more information.

(file #44501)

 Mike Miller <mtmiller>
Group Member
Mon 02 Jul 2018 09:44:13 PM UTC, comment #1: 

Attaching the patch I provided in bug #54217 as a workaround to compile pr-output.cc on Solaris.

If we need to keep this hack, maybe a combination of renaming the function and declaring it for only the sized integer types (int8, int16, etc) would fix both of these problems, untested.

(file #44496)

 Mike Miller <mtmiller>
Group Member
Mon 02 Jul 2018 09:41:55 PM UTC, original submission:  

The local 'abs' function template in pr-output.cc and marked with a 'FIXME' comment is not a good general or portable solution. The intent here is to create explicit 'abs' overrides for all signed and unsigned integer types to avoid a GCC warning message about comparison between signed and unsigned types.

In execution, this local 'abs' declaration has a few problems:

  • Building with the Clang compiler on 64-bit GNU/Linux produces "warning: unused function 'abs'" on the 'SPECIALIZE_UABS(long long)' template instantiation, see bug #53650. On 64-bit systems, 'long long' and 'unsigned long long' are never used by Octave.


  • Building with GCC 4.9.2 on Solaris 10 fails to compile with an error message "error: call of overloaded ‘abs(short int)’ is ambiguous", and likewise for other integer types. For some reason the compiler is unable to declare that the local template overloads for integer types are better than the standard library floating-point 'abs' functions. See bug #54217 for the full error message. Changing the names of the local function templates to 'xabs' fixed this error.
Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #44501:  bug54237.diff added by mtmiller (4KiB - text/x-patch)
file #44496:  abs.diff added by mtmiller (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by michele31415 (Posted a comment)
  • -email is unavailable- added by mtmiller
  • -email is unavailable- added by mtmiller (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 group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-10 mtmiller StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2018-07-03 mtmiller Carbon-Copy- Added michele31415
    2018-07-03 mtmiller Attached File- Added bug54237.diff, #44501
        StatusPostponed Patch Submitted
    2018-07-02 mtmiller Attached File- Added abs.diff, #44496

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.14-f13d.
    Corresponding source code