Merge pull request #53 from isuruf/fixes

Fixes

Author: isuruf

.travis.yml

@@ -13,6 +13,7 @@ addons:
     - libmpc-dev
     - binutils-dev
     - g++-4.7
+
 rvm:
  - 2.0
  - 2.1
@@ -23,6 +24,15 @@ matrix:
   exclude:
     - os: osx
   include:
+    - os: linux
+      rvm: 2.0
+      env: BUILD_TYPE="Debug" WITH_BFD="yes" WITH_MPC="yes"
+    - os: linux
+      rvm: 2.3.0
+      env: BUILD_TYPE="Debug" WITH_BFD="yes" WITH_MPC="yes"
+    - os: linux
+      rvm: 2.3.0
+      env: BUILD_TYPE="Debug" WITH_BFD="yes" WITH_MPFR="yes"
     - os: osx
       rvm: 2.0
   allow_failures:
@@ -52,6 +62,8 @@ install:
 script:
   # Build ruby gem
   - gem build symengine.gemspec
+  # Convert all warnings to errors
+  - export CFLAGS="-Werror"
   # Install ruby gem
   - gem install symengine-0.1.0.gem --install-dir $HOME --verbose -- -DSymEngine_DIR=$our_install_dir
 

CMakeLists.txt

@@ -7,8 +7,8 @@ set(CMAKE_PREFIX_PATH ${SymEngine_DIR} ${CMAKE_PREFIX_PATH})
 find_package(SymEngine 0.1.0 REQUIRED CONFIG
              PATH_SUFFIXES lib/cmake/symengine CMake/)
 set(CMAKE_BUILD_TYPE ${SYMENGINE_BUILD_TYPE})
-set(CMAKE_CXX_FLAGS_RELEASE ${SYMENGINE_CXX_FLAGS_RELEASE})
-set(CMAKE_CXX_FLAGS_DEBUG ${SYMENGINE_CXX_FLAGS_DEBUG})
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall")
+message("SymEngine directory: ${SymEngine_DIR}")
 include_directories(${SYMENGINE_INCLUDE_DIRS})
 
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/")

ext/symengine/ruby_basic.c

@@ -90,12 +90,15 @@ VALUE cbasic_diff(VALUE self, VALUE operand2) {
 }
 
 VALUE cbasic_eq(VALUE self, VALUE operand2) {
+
     basic_struct *this;
 
     basic cbasic_operand2;
     basic_new_stack(cbasic_operand2);
     Data_Get_Struct(self, basic_struct, this);
-    sympify(operand2, cbasic_operand2);
+
+    VALUE ret = check_sympify(operand2, cbasic_operand2);
+    if (ret == Qfalse) return Qfalse;
 
     VALUE ret_val = basic_eq(this, cbasic_operand2) ? Qtrue : Qfalse;
     basic_free_stack(cbasic_operand2);
@@ -109,7 +112,9 @@ VALUE cbasic_neq(VALUE self, VALUE operand2) {
     basic cbasic_operand2;
     basic_new_stack(cbasic_operand2);
     Data_Get_Struct(self, basic_struct, this);
-    sympify(operand2, cbasic_operand2);
+
+    VALUE ret = check_sympify(operand2, cbasic_operand2);
+    if (ret == Qfalse) return Qtrue;
 
     VALUE ret_val =  basic_neq(this, cbasic_operand2) ? Qtrue : Qfalse;
     basic_free_stack(cbasic_operand2);
@@ -122,7 +127,7 @@ VALUE cbasic_neg(VALUE self){
 }
 
 VALUE cbasic_get_args(VALUE self) {
-    basic_struct *this, *iterator_basic;
+    basic_struct *this;
     CVecBasic *args = vecbasic_new();
     int size = 0;
 
@@ -132,7 +137,7 @@ VALUE cbasic_get_args(VALUE self) {
     size = vecbasic_size(args);
     VALUE ruby_array = rb_ary_new2(size);
     int i = 0;
-    VALUE temp = NULL;
+    VALUE temp;
     for(i = 0; i < size; i++) {
         basic_struct *temp_basic = basic_new_heap();
         vecbasic_get(args, i, temp_basic);
@@ -144,7 +149,7 @@ VALUE cbasic_get_args(VALUE self) {
 }
 
 VALUE cbasic_free_symbols(VALUE self) {
-    basic_struct *this, *iterator_basic;
+    basic_struct *this;
     CSetBasic *symbols = setbasic_new();
     int size = 0;
 
@@ -154,7 +159,7 @@ VALUE cbasic_free_symbols(VALUE self) {
     size = setbasic_size(symbols);
     VALUE ruby_array = rb_ary_new2(size);
     int i = 0;
-    VALUE temp = NULL;
+    VALUE temp;
     for(i = 0; i < size; i++) {
         basic_struct *temp_basic = basic_new_heap();
         setbasic_get(symbols, i, temp_basic);

ext/symengine/ruby_real_mpfr.c

@@ -20,8 +20,7 @@ VALUE crealmpfr_init(VALUE self, VALUE num_value, VALUE prec_value)
             real_mpfr_set_str(cresult, c, prec);
             break;
         case T_DATA:
-            c = rb_obj_classname(num_value);
-            if (strcmp(c, "BigDecimal") == 0) {
+            if (strcmp(rb_obj_classname(num_value), "BigDecimal") == 0) {
                 c = RSTRING_PTR(rb_funcall(num_value, rb_intern("to_s"), 1, rb_str_new2("F")));
                 real_mpfr_set_str(cresult, c, prec);
                 break;

ext/symengine/ruby_symbol.c

@@ -1,17 +1,17 @@
 #include "ruby_symbol.h"
 
 VALUE csymbol_init(VALUE self, VALUE name_or_id) {
-    char *str_ptr;
+    const char *str_ptr;
 
     switch (TYPE(name_or_id)) {
-    case T_STRING:
-      str_ptr = StringValueCStr(name_or_id);
-      break;
-    case T_SYMBOL:
-      str_ptr = rb_id2name(rb_to_id(name_or_id));
-      break;      
-    default:
-      rb_raise(rb_eTypeError, "wrong argument type %s (expected Symbol or String)", rb_obj_classname(name_or_id));
+        case T_STRING:
+            str_ptr = StringValueCStr(name_or_id);
+            break;
+        case T_SYMBOL:
+            str_ptr = rb_id2name(rb_to_id(name_or_id));
+            break;      
+        default:
+            rb_raise(rb_eTypeError, "wrong argument type %s (expected Symbol or String)", rb_obj_classname(name_or_id));
     }
 
     basic_struct *this;

ext/symengine/symengine.c

@@ -207,4 +207,5 @@ void Init_symengine() {
     rb_define_module_function(m_symengine, "lucas", cntheory_lucas, 1);
     rb_define_module_function(m_symengine, "binomial", cntheory_binomial, 2);
 
+    symengine_print_stack_on_segfault();
 }

ext/symengine/symengine.h

@@ -1,7 +1,8 @@
 #ifndef SYMENGINE_H_
 #define SYMENGINE_H_
 
-#include "ruby.h"
+#include <ruby/ruby.h>
+#include <ruby/intern.h>
 
 //variable name for a module starts with m
 VALUE m_symengine;

ext/symengine/symengine_utils.c

@@ -1,13 +1,11 @@
 #include "symengine_utils.h"
 #include "symengine.h"
 
-void sympify(VALUE operand2, basic_struct *cbasic_operand2) {
+VALUE check_sympify(VALUE operand2, basic_struct *cbasic_operand2) {
 
     basic_struct *temp;
-    VALUE new_operand2;
     VALUE a, b;
     double f;
-    char *c;
 
     switch(TYPE(operand2)) {
         case T_FIXNUM:
@@ -60,18 +58,29 @@ void sympify(VALUE operand2, basic_struct *cbasic_operand2) {
             break;
 
         case T_DATA:
-            c = rb_obj_classname(operand2);
+            if (rb_obj_is_kind_of(operand2, c_basic)) {
+                Data_Get_Struct(operand2, basic_struct, temp);
+                basic_assign(cbasic_operand2, temp);
+                break;
+            }
             #ifdef HAVE_SYMENGINE_MPFR
-            if (strcmp(c, "BigDecimal") == 0) {
+            if (strcmp(rb_obj_classname(operand2), "BigDecimal") == 0) {
+                const char *c;
                 c = RSTRING_PTR( rb_funcall(operand2, rb_intern("to_s"), 1, rb_str_new2("F")) );
                 real_mpfr_set_str(cbasic_operand2, c, 200);
                 break;
             }
             #endif //HAVE_SYMENGINE_MPFR
+        default:
+            return Qfalse;
+    }
+    return Qtrue;
+}
 
-            Data_Get_Struct(operand2, basic_struct, temp);
-            basic_assign(cbasic_operand2, temp);
-            break;
+void sympify(VALUE operand2, basic_struct *cbasic_operand2) {
+    VALUE ret = check_sympify(operand2, cbasic_operand2);
+    if (ret == Qfalse) {
+        rb_raise(rb_eTypeError, "%s can't be coerced into SymEngine::Basic", rb_obj_classname(operand2));
     }
 }
 

ext/symengine/symengine_utils.h

@@ -7,8 +7,10 @@
 
 VALUE rb_cBigDecimal;
 
-//Returns the pointer wrapped inside the Ruby VALUE
+//Coerces operand2 into a SymEngine object
 void sympify(VALUE operand2, basic_struct *cbasic_operand2);
+//Coerces operand2 into a SymEngine object and returns whether successful
+VALUE check_sympify(VALUE operand2, basic_struct *cbasic_operand2);
 //Returns the pointer wrapped inside the Ruby Fixnum or Bignum 
 void get_symintfromval(VALUE operand2, basic_struct *cbasic_operand2);
 //Returns the Ruby class of the corresponding basic_struct pointer

spec/basic_spec.rb

@@ -60,6 +60,7 @@
       subject { x * y }
       it { is_expected.to eq sym('x')*sym('y') }
       it { is_expected.not_to eq sym('x')*sym('z') }
+      it { is_expected.not_to eq "asd" }
     end
 
     it 'simplifies' do
@@ -90,6 +91,7 @@
 
     it 'raises on wrong args' do
       expect { 'x' * x }.to raise_error(TypeError)
+      expect {  x  * 'x' }.to raise_error(TypeError)
     end
   end