Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused functions from ccobject.h #24153

Closed
jdemeyer opened this issue Nov 3, 2017 · 7 comments
Closed

Remove unused functions from ccobject.h #24153

jdemeyer opened this issue Nov 3, 2017 · 7 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Nov 3, 2017

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

CC: @tscrim

Component: cython

Author: Jeroen Demeyer

Branch/Commit: fdffc30

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/24153

@jdemeyer jdemeyer added this to the sage-8.1 milestone Nov 3, 2017
@jdemeyer
Copy link
Author

jdemeyer commented Nov 3, 2017

Branch: u/jdemeyer/ticket/24153

@jdemeyer
Copy link
Author

jdemeyer commented Nov 3, 2017

Commit: fdffc30

@jdemeyer
Copy link
Author

jdemeyer commented Nov 3, 2017

New commits:

fdffc30Remove unused functions from ccobject.h

@tscrim
Copy link
Collaborator

tscrim commented Nov 3, 2017

comment:3

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:

diff --git a/src/sage/ext/ccobject.h b/src/sage/ext/ccobject.h
index 6753a0a..c6cadb9 100644
--- a/src/sage/ext/ccobject.h
+++ b/src/sage/ext/ccobject.h
@@ -114,15 +49,18 @@ static CYTHON_INLINE PyObject* ccrepr(const T& x)
 #endif
 }
 
+
 /* Arrays */
 template <class T>
-static inline T* Allocate_array(size_t n){
-  return new T[n];
+static inline T* Allocate_array(size_t n)
+{
+    return new T[n];
 }
 
 template <class T>
-static inline void Delete_array(T* v){
-  delete[] v;
+static inline void Delete_array(T* v)
+{
+    delete[] v;
 }
 
 #endif

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.

@tscrim
Copy link
Collaborator

tscrim commented Nov 3, 2017

Reviewer: Travis Scrimshaw

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2017

comment:4

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.

@vbraun
Copy link
Member

vbraun commented Dec 11, 2017

Changed branch from u/jdemeyer/ticket/24153 to fdffc30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants