Opened 5 years ago

Closed 5 years ago

#24153 closed enhancement (fixed)

Remove unused functions from ccobject.h

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-8.1
Component: cython Keywords:
Cc: Travis Scrimshaw Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: fdffc30 (Commits, GitHub, GitLab) Commit: fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb
Dependencies: Stopgaps:

Status badges


Thanks to various C++ cleanup tickets, most of ccobject.h is obsolete now.

Change History (5)

comment:1 Changed 5 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/24153

comment:2 Changed 5 years ago by Jeroen Demeyer

Commit: fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb
Status: newneeds_review

New commits:

fdffc30Remove unused functions from ccobject.h

comment:3 Changed 5 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw

I know this is bikeshedding, nobody will probably ever really read this file, and C++ does not have nearly the same style uniformity that Python has. So you can completely ignore this. Yet, IMO, most C++ programmers have their opening { on the same line that starts the block (and it is more Python-esque). So I do not quite agree with this:

  • src/sage/ext/ccobject.h

    diff --git a/src/sage/ext/ccobject.h b/src/sage/ext/ccobject.h
    index 6753a0a..c6cadb9 100644
    a b static CYTHON_INLINE PyObject* ccrepr(const T& x) 
    11753/* Arrays */
    11854template <class T>
    119 static inline T* Allocate_array(size_t n){
    120   return new T[n];
     55static inline T* Allocate_array(size_t n)
     57    return new T[n];
    12360template <class T>
    124 static inline void Delete_array(T* v){
    125   delete[] v;
     61static inline void Delete_array(T* v)
     63    delete[] v;

However, if you prefer it in even the slightest (including out of laziness), then just set a positive review. Likewise, if you undo this change, you can set a positive review on my behalf.

Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:4 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewpositive_review

Google agrees with you:

GNU agrees with me:

I'll go for the "lazy" option and keep this branch as is.

comment:5 Changed 5 years ago by Volker Braun

Branch: u/jdemeyer/ticket/24153fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.