Re: Issue 4152: Eliminate "will never be null" warnings in smobs.tcc (issue 153970044 by address@hidden)

[dak]

Reviewers: Dan Eble,


https://codereview.appspot.com/153970044/diff/1/lily/include/smobs.hh
File lily/include/smobs.hh (right):

https://codereview.appspot.com/153970044/diff/1/lily/include/smobs.hh#newcode175
lily/include/smobs.hh:175: // Since we consider those internal-only, two
of them are actually
On 2014/10/04 13:19:35, Dan Eble wrote:
> Update comment?

Acknowledged.

Description:
Issue 4152: Eliminate "will never be null" warnings in smobs.tcc

Hopefully this will no longer cause spurious warnings with g++ 4.7.

Please review this at https://codereview.appspot.com/153970044/

Affected files (+35, -17 lines):
  M lily/include/small-smobs.hh
  M lily/include/smobs.hh
  M lily/include/smobs.tcc


Index: lily/include/small-smobs.hh
diff --git a/lily/include/small-smobs.hh b/lily/include/small-smobs.hh
index  
a752662c1e534da09c82336d8be22b7bd71f4e69..fa63af9a90057672fb18c86df2c17c55dcad7919  
100644
--- a/lily/include/small-smobs.hh
+++ b/lily/include/small-smobs.hh
@@ -32,7 +32,6 @@ public:
   static SCM make_smob (SCM arg1 = SCM_UNDEFINED) {
     SCM_RETURN_NEWSMOB (Smob_base<Super>::smob_tag (), SCM_UNPACK (arg1));
   }
-  static const int free_smob = 0;
   SCM mark_smob () { return scm1 (); };
   static Super *unchecked_unsmob (SCM s) {
     return reinterpret_cast<Super *> (SCM_UNPACK (s));
@@ -53,7 +52,6 @@ public:
                          SCM_UNPACK (arg1),
                          SCM_UNPACK (arg2));
   }
-  static const int free_smob = 0;
   SCM mark_smob ()
   {
     scm_gc_mark (scm2 ());
@@ -82,7 +80,6 @@ public:
                          SCM_UNPACK (arg2),
                          SCM_UNPACK (arg3));
   }
-  static const int free_smob = 0;
   static SCM mark_smob (SCM s)
   {
     scm_gc_mark (scm3 ());
Index: lily/include/smobs.hh
diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh
index  
f9eeed7f85545e8dc9a5b4c539129bd51a09178c..979d329667f85d91a5ce4bd362930e073499a16a  
100644
--- a/lily/include/smobs.hh
+++ b/lily/include/smobs.hh
@@ -182,15 +182,11 @@ private:
   SCM mark_smob (void); // Should not be inline since we do an address
                         // comparison
   static SCM mark_trampoline (SCM); // Used for calling mark_smob
-  static const int equal_p = 0;
+  static size_t free_smob (SCM obj);
+  static SCM equal_p (SCM, SCM);
   static const int smob_proc = 0;
-  static const int smob_proc_signature_ = 0;
+  static const int smob_proc_signature_ = -1;
   static int print_smob (SCM, SCM, scm_print_state *);
-  static size_t free_smob (SCM obj)
-  {
-    delete Smob_base<Super>::unregister_ptr (obj);
-    return 0;
-  }
   // type_p_name_ can be overriden in the Super class with a static
   // const char [] string.  This requires both a declaration in the
   // class as well as a single instantiation outside.  Using a
@@ -232,10 +228,15 @@ public:
   }
 };

-
+// Simple smobs
 template <class Super>
 class Simple_smob : public Smob_base<Super> {
 public:
+  static size_t free_smob (SCM obj)
+  {
+    delete Smob_base<Super>::unregister_ptr (obj);
+    return 0;
+  }
   SCM smobbed_copy () const
   {
     Super *p = new Super(*static_cast<const Super *> (this));
@@ -252,6 +253,11 @@ private:
   SCM self_scm_;
   SCM protection_cons_;
 public:
+  static size_t free_smob (SCM obj)
+  {
+    delete Smob_base<Super>::unregister_ptr (obj);
+    return 0;
+  }
   SCM unprotected_smobify_self ()
   {
     self_scm_ = SCM_UNDEFINED;
Index: lily/include/smobs.tcc
diff --git a/lily/include/smobs.tcc b/lily/include/smobs.tcc
index  
c2616b321692ec431f4c0dea55370872b0ab976f..eb39c0ca698a9995e6e98f24ebc22f4c95e66a62  
100644
--- a/lily/include/smobs.tcc
+++ b/lily/include/smobs.tcc
@@ -49,7 +49,7 @@ Smob_base<Super>::register_ptr (Super *p)
   return s;
 }

-// Default, should not actually get called
+// Defaults, should not actually get called
 template <class Super>
 SCM
 Smob_base<Super>::mark_smob ()
@@ -58,6 +58,22 @@ Smob_base<Super>::mark_smob ()
 }

 template <class Super>
+size_t
+Smob_base<Super>::free_smob (SCM)
+{
+  return 0;
+}
+
+template <class Super>
+SCM
+Smob_base<Super>::equal_p (SCM, SCM)
+{
+  return SCM_BOOL_F;
+}
+
+// Default, will often get called
+
+template <class Super>
 int
 Smob_base<Super>::print_smob (SCM, SCM p, scm_print_state *)
 {
@@ -103,13 +119,12 @@ void Smob_base<Super>::init ()
   // While that's not a consideration for type_p_name_, it's easier
   // doing it like the rest.

-  if (Super::free_smob != 0)
+  if (&Super::free_smob != &Smob_base<Super>::free_smob)
     scm_set_smob_free (smob_tag_, Super::free_smob);
   if (&Super::mark_smob != &Smob_base<Super>::mark_smob)
     scm_set_smob_mark (smob_tag_, Super::mark_trampoline);
-  if (Super::print_smob != 0)
-    scm_set_smob_print (smob_tag_, Super::print_smob);
-  if (Super::equal_p != 0)
+  scm_set_smob_print (smob_tag_, Super::print_smob);
+  if (&Super::equal_p != &Smob_base<Super>::equal_p)
     scm_set_smob_equalp (smob_tag_, Super::equal_p);
   if (Super::type_p_name_ != 0)
     {
@@ -122,7 +137,7 @@ void Smob_base<Super>::init ()
       scm_c_export (Super::type_p_name_, NULL);
     }
   ly_add_type_predicate ((void *) unsmob, smob_name_.c_str ());
-  if (Super::smob_proc != 0)
+  if (Super::smob_proc_signature_ >= 0)
     scm_set_smob_apply (smob_tag_,
                         (scm_t_subr)Super::smob_proc,
                         Super::smob_proc_signature_ >> 8,