From d92b6d8120a880f184a4545ae5cce7d40343cce5 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Tue, 14 Jun 2016 22:04:52 +0530 Subject: [PATCH 01/11] Fix warnings --- ext/symengine/ruby_basic.c | 4 ++-- ext/symengine/ruby_real_mpfr.c | 3 +-- ext/symengine/ruby_symbol.c | 18 +++++++++--------- ext/symengine/symengine_utils.c | 3 +-- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/ext/symengine/ruby_basic.c b/ext/symengine/ruby_basic.c index 80cabd3..8442345 100644 --- a/ext/symengine/ruby_basic.c +++ b/ext/symengine/ruby_basic.c @@ -132,7 +132,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); @@ -154,7 +154,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); diff --git a/ext/symengine/ruby_real_mpfr.c b/ext/symengine/ruby_real_mpfr.c index d97953c..5fe8666 100644 --- a/ext/symengine/ruby_real_mpfr.c +++ b/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; diff --git a/ext/symengine/ruby_symbol.c b/ext/symengine/ruby_symbol.c index 7e3bb47..e48a1df 100644 --- a/ext/symengine/ruby_symbol.c +++ b/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; diff --git a/ext/symengine/symengine_utils.c b/ext/symengine/symengine_utils.c index e4385f1..8a587f1 100644 --- a/ext/symengine/symengine_utils.c +++ b/ext/symengine/symengine_utils.c @@ -60,9 +60,8 @@ void sympify(VALUE operand2, basic_struct *cbasic_operand2) { break; case T_DATA: - c = rb_obj_classname(operand2); #ifdef HAVE_SYMENGINE_MPFR - if (strcmp(c, "BigDecimal") == 0) { + if (strcmp(rb_obj_classname(operand2), "BigDecimal") == 0) { c = RSTRING_PTR( rb_funcall(operand2, rb_intern("to_s"), 1, rb_str_new2("F")) ); real_mpfr_set_str(cbasic_operand2, c, 200); break; From df3c9d594ec5773f9029eb757c35baa79ea88122 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Tue, 14 Jun 2016 22:10:36 +0530 Subject: [PATCH 02/11] Convert all warnings to errors --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 86709ff..b1ee653 100644 --- a/.travis.yml +++ b/.travis.yml @@ -52,6 +52,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 From 313208de4269fc81fee50ed4aa17b77834ab48e6 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Tue, 14 Jun 2016 22:40:59 +0530 Subject: [PATCH 03/11] Print stack on segfault --- ext/symengine/symengine.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/symengine/symengine.c b/ext/symengine/symengine.c index f86969e..cb7e15e 100644 --- a/ext/symengine/symengine.c +++ b/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(); } From b2af1e1d624ec7838852642bc4134fdbf2927e7a Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Tue, 14 Jun 2016 23:15:28 +0530 Subject: [PATCH 04/11] Fail fast if sympify fails --- ext/symengine/symengine.h | 3 ++- ext/symengine/symengine_utils.c | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ext/symengine/symengine.h b/ext/symengine/symengine.h index f850d57..35a5947 100644 --- a/ext/symengine/symengine.h +++ b/ext/symengine/symengine.h @@ -1,7 +1,8 @@ #ifndef SYMENGINE_H_ #define SYMENGINE_H_ -#include "ruby.h" +#include +#include //variable name for a module starts with m VALUE m_symengine; diff --git a/ext/symengine/symengine_utils.c b/ext/symengine/symengine_utils.c index 8a587f1..88b686a 100644 --- a/ext/symengine/symengine_utils.c +++ b/ext/symengine/symengine_utils.c @@ -60,6 +60,11 @@ void sympify(VALUE operand2, basic_struct *cbasic_operand2) { break; case T_DATA: + 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(rb_obj_classname(operand2), "BigDecimal") == 0) { c = RSTRING_PTR( rb_funcall(operand2, rb_intern("to_s"), 1, rb_str_new2("F")) ); @@ -67,10 +72,8 @@ void sympify(VALUE operand2, basic_struct *cbasic_operand2) { break; } #endif //HAVE_SYMENGINE_MPFR - - Data_Get_Struct(operand2, basic_struct, temp); - basic_assign(cbasic_operand2, temp); - break; + default: + rb_raise(rb_eTypeError, "%s can't be coerced into SymEngine::Basic", rb_obj_classname(operand2)); } } From 153a31c3a913d2a3e1aa9c0ca299bbdcbaddb906 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Tue, 14 Jun 2016 23:54:42 +0530 Subject: [PATCH 05/11] basic == unknown object return false now --- ext/symengine/ruby_basic.c | 9 +++++++-- ext/symengine/symengine_utils.c | 14 +++++++++++--- ext/symengine/symengine_utils.h | 4 +++- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/ext/symengine/ruby_basic.c b/ext/symengine/ruby_basic.c index 8442345..6890591 100644 --- a/ext/symengine/ruby_basic.c +++ b/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); diff --git a/ext/symengine/symengine_utils.c b/ext/symengine/symengine_utils.c index 88b686a..56ff154 100644 --- a/ext/symengine/symengine_utils.c +++ b/ext/symengine/symengine_utils.c @@ -1,13 +1,13 @@ #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; + const char *c; switch(TYPE(operand2)) { case T_FIXNUM: @@ -73,7 +73,15 @@ void sympify(VALUE operand2, basic_struct *cbasic_operand2) { } #endif //HAVE_SYMENGINE_MPFR default: - rb_raise(rb_eTypeError, "%s can't be coerced into SymEngine::Basic", rb_obj_classname(operand2)); + return Qfalse; + } + return Qtrue; +} + +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)); } } diff --git a/ext/symengine/symengine_utils.h b/ext/symengine/symengine_utils.h index 2910464..b5d4b2b 100644 --- a/ext/symengine/symengine_utils.h +++ b/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 From 8dae6244ede03506329b525bc005ebb175cdb6e4 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 00:18:58 +0530 Subject: [PATCH 06/11] Travis test with mpc and debug --- .travis.yml | 10 ++++++++++ symengine_version.txt | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index b1ee653..50aa917 100644 --- a/.travis.yml +++ b/.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: diff --git a/symengine_version.txt b/symengine_version.txt index e83d8de..3eba091 100644 --- a/symengine_version.txt +++ b/symengine_version.txt @@ -1 +1 @@ -0086007c3f588e77c9b8ce8e071fee686ffc6b72 +a106bdd1503ee1c15a381f3fa6684fde466c1ddd From 893257d8e95f2f3dea24bcf317d81ea9214f3a9c Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 00:38:24 +0530 Subject: [PATCH 07/11] Add -Wall to C flags --- CMakeLists.txt | 3 +-- ext/symengine/ruby_basic.c | 4 ++-- ext/symengine/symengine_utils.c | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9ac6c72..1eb79c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,8 +7,7 @@ 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") include_directories(${SYMENGINE_INCLUDE_DIRS}) set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/") diff --git a/ext/symengine/ruby_basic.c b/ext/symengine/ruby_basic.c index 6890591..8630615 100644 --- a/ext/symengine/ruby_basic.c +++ b/ext/symengine/ruby_basic.c @@ -127,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; @@ -149,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; diff --git a/ext/symengine/symengine_utils.c b/ext/symengine/symengine_utils.c index 56ff154..3e593c7 100644 --- a/ext/symengine/symengine_utils.c +++ b/ext/symengine/symengine_utils.c @@ -4,7 +4,6 @@ VALUE check_sympify(VALUE operand2, basic_struct *cbasic_operand2) { basic_struct *temp; - VALUE new_operand2; VALUE a, b; double f; const char *c; From aaa20cc45740f58b875f2993350b08a5600f5b6d Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 00:48:34 +0530 Subject: [PATCH 08/11] Add tests for coercion error and equals with incompatible types --- spec/basic_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/basic_spec.rb b/spec/basic_spec.rb index e70bd64..c257478 100644 --- a/spec/basic_spec.rb +++ b/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 From bde10244a8a722707a82965bb3f7696da60db753 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 01:07:04 +0530 Subject: [PATCH 09/11] Print SymEngine directory --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1eb79c9..1961590 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,6 +8,7 @@ find_package(SymEngine 0.1.0 REQUIRED CONFIG PATH_SUFFIXES lib/cmake/symengine CMake/) set(CMAKE_BUILD_TYPE ${SYMENGINE_BUILD_TYPE}) 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/") From b05353334e4358d712f65b380d413421fe7f73c9 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 01:08:21 +0530 Subject: [PATCH 10/11] Fix unused variable warning --- ext/symengine/symengine_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/symengine/symengine_utils.c b/ext/symengine/symengine_utils.c index 3e593c7..9ea19e5 100644 --- a/ext/symengine/symengine_utils.c +++ b/ext/symengine/symengine_utils.c @@ -6,7 +6,6 @@ VALUE check_sympify(VALUE operand2, basic_struct *cbasic_operand2) { basic_struct *temp; VALUE a, b; double f; - const char *c; switch(TYPE(operand2)) { case T_FIXNUM: @@ -66,6 +65,7 @@ VALUE check_sympify(VALUE operand2, basic_struct *cbasic_operand2) { } #ifdef HAVE_SYMENGINE_MPFR 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; From ca25263455a751909340e9bf800c546ed00301d8 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Wed, 15 Jun 2016 07:46:25 +0530 Subject: [PATCH 11/11] Update symengine version --- symengine_version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symengine_version.txt b/symengine_version.txt index 3eba091..bc0c91d 100644 --- a/symengine_version.txt +++ b/symengine_version.txt @@ -1 +1 @@ -a106bdd1503ee1c15a381f3fa6684fde466c1ddd +0c17ca9d88f25881c6d0ca382c26b2eb392b9b48