Opened 4 years ago

Closed 4 years ago

#24153 closed enhancement (fixed)

Remove unused functions from ccobject.h

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.1
Component: cython Keywords:
Cc: tscrim 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

Description

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

Change History (5)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/24153

comment:2 Changed 4 years ago by jdemeyer

  • Commit set to fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb
  • Status changed from new to needs_review

New commits:

fdffc30Remove unused functions from ccobject.h

comment:3 Changed 4 years ago by tscrim

  • Reviewers set to 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) 
    11449#endif
    11550}
    11651
     52
    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)
     56{
     57    return new T[n];
    12158}
    12259
    12360template <class T>
    124 static inline void Delete_array(T* v){
    125   delete[] v;
     61static inline void Delete_array(T* v)
     62{
     63    delete[] v;
    12664}
    12765
    12866#endif

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

Version 0, edited 4 years ago by tscrim (next)

comment:4 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Google agrees with you: http://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions

GNU agrees with me: https://www.gnu.org/prep/standards/standards.html#Formatting

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

comment:5 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/24153 to fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.