Skip to content

Additional Fixes for #1139#1141

Merged
isuruf merged 6 commits intosymengine:masterfrom
ShikharJ:Issue1139
Dec 9, 2016
Merged

Additional Fixes for #1139#1141
isuruf merged 6 commits intosymengine:masterfrom
ShikharJ:Issue1139

Conversation

@ShikharJ
Copy link
Copy Markdown
Member

@ShikharJ ShikharJ commented Dec 2, 2016

Some more simple fixes for #1139.
The commits currently cover:

cwrapper.cpp
test_basic.cpp
test_parser.cpp
test_functions.cpp
test_printing.cpp
test_number.cpp

Additionally, I feel that our use of static_cast() in the following files is essential, as only type conversions are done there, (and no Base-to-Derived or Derived-to-Base pointer changes):

basic-inl.h
test_evalf.cpp

Copy link
Copy Markdown
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!

@ShikharJ ShikharJ force-pushed the Issue1139 branch 2 times, most recently from 6823063 to 95c6ea2 Compare December 5, 2016 08:39
@ShikharJ
Copy link
Copy Markdown
Member Author

ShikharJ commented Dec 5, 2016

@isuruf @certik There are still around 30 or so remaining instances of static_cast() use in the symengine directory (apart from the utilities) after these commits. Some 5-6 are cases of type conversions, whereas the rest are raising issues (I have individually tried every one of them). Some example errors are:

In file included from /home/shikhar/symengine/symengine/symengine_rcp.h:11:0,
                 from /home/shikhar/symengine/symengine/mp_wrapper.h:4,
                 from /home/shikhar/symengine/symengine/mp_class.h:5,
                 from /home/shikhar/symengine/symengine/dict.h:9,
                 from /home/shikhar/symengine/symengine/basic.h:32,
                 from /home/shikhar/symengine/symengine/polys/upolybase.h:8,
                 from /home/shikhar/symengine/symengine/polys/uintpoly_flint.h:8,
                 from /home/shikhar/symengine/symengine/visitor.h:9,
                 from /home/shikhar/symengine/symengine/printer.h:4,
                 from /home/shikhar/symengine/symengine/tests/polynomial/test_mexprpoly.cpp:4:
/home/shikhar/symengine/symengine/symengine_casts.h: In instantiation of ‘To SymEngine::down_cast(From&) [with To = SymEngine::MExprDict&; From = SymEngine::UDictWrapper<std::vector<int>, SymEngine::Expression, SymEngine::MExprDict>]’:
/home/shikhar/symengine/symengine/polys/msymenginepoly.h:131:36:   required from ‘Wrapper& SymEngine::UDictWrapper<Vec, Value, Wrapper>::operator-=(const Wrapper&) [with Vec = std::vector<int>; Value = SymEngine::Expression; Wrapper = SymEngine::MExprDict]’
/home/shikhar/symengine/symengine/polys/msymenginepoly.h:520:7:   required from ‘Teuchos::RCP<const T> SymEngine::sub_mpoly(const Poly&, const Poly&) [with Poly = SymEngine::MExprPoly]’
/home/shikhar/symengine/symengine/tests/polynomial/test_mexprpoly.cpp:365:5:   required from here
/home/shikhar/symengine/symengine/symengine_casts.h:85:52: error: cannot dynamic_cast ‘& f’ (of type ‘class SymEngine::UDictWrapper<std::vector<int>, SymEngine::Expression, SymEngine::MExprDict>*’) to type ‘ToAsPointer {aka class SymEngine::MExprDict*}’ (source type is not polymorphic)
     SYMENGINE_ASSERT(dynamic_cast<ToAsPointer>(&f) != NULL);
                                                    ^
/home/shikhar/symengine/symengine/symengine_assert.h:12:19: note: in definition of macro ‘SYMENGINE_ASSERT’
         if (0 == (cond)) {                                                     \
                   ^
symengine/tests/polynomial/CMakeFiles/test_mexprpoly.dir/build.make:62: recipe for target 'symengine/tests/polynomial/CMakeFiles/test_mexprpoly.dir/test_mexprpoly.cpp.o' failed
make[2]: *** [symengine/tests/polynomial/CMakeFiles/test_mexprpoly.dir/test_mexprpoly.cpp.o] Error 1
CMakeFiles/Makefile2:1503: recipe for target 'symengine/tests/polynomial/CMakeFiles/test_mexprpoly.dir/all' failed
make[1]: *** [symengine/tests/polynomial/CMakeFiles/test_mexprpoly.dir/all] Error 2
Makefile:127: recipe for target 'all' failed
make: *** [all] Error 2
In file included from /home/shikhar/symengine/symengine/symengine_rcp.h:11:0,
                 from /home/shikhar/symengine/symengine/mp_wrapper.h:4,
                 from /home/shikhar/symengine/symengine/mp_class.h:5,
                 from /home/shikhar/symengine/symengine/dict.h:9,
                 from /home/shikhar/symengine/symengine/basic.h:32,
                 from /home/shikhar/symengine/symengine/fields.h:8,
                 from /home/shikhar/symengine/symengine/fields.cpp:1:
/home/shikhar/symengine/symengine/symengine_casts.h: In instantiation of ‘To SymEngine::down_cast(From&) [with To = SymEngine::GaloisFieldDict; From = const SymEngine::GaloisFieldDict]’:
/home/shikhar/symengine/symengine/fields.cpp:287:48:   required from here
/home/shikhar/symengine/symengine/symengine_casts.h:85:52: error: cannot dynamic_cast ‘& f’ (of type ‘const class SymEngine::GaloisFieldDict*’) to type ‘ToAsPointer {aka class SymEngine::GaloisFieldDict*}’ (conversion casts away constness)
     SYMENGINE_ASSERT(dynamic_cast<ToAsPointer>(&f) != NULL);
                                                    ^
/home/shikhar/symengine/symengine/symengine_assert.h:12:19: note: in definition of macro ‘SYMENGINE_ASSERT’
         if (0 == (cond)) {                                                     \
                   ^
In file included from /home/shikhar/symengine/symengine/functions.h:11:0,
                 from /home/shikhar/symengine/symengine/pow.h:9,
                 from /home/shikhar/symengine/symengine/polys/upolybase.h:9,
                 from /home/shikhar/symengine/symengine/fields.h:10,
                 from /home/shikhar/symengine/symengine/fields.cpp:1:
/home/shikhar/symengine/symengine/symengine_casts.h:87:40: error: static_cast from type ‘const SymEngine::GaloisFieldDict*’ to type ‘ToAsPointer {aka SymEngine::GaloisFieldDict*}’ casts away qualifiers
     return *static_cast<ToAsPointer>(&f);
                                        ^
symengine/CMakeFiles/symengine.dir/build.make:1083: recipe for target 'symengine/CMakeFiles/symengine.dir/fields.cpp.o' failed
make[2]: *** [symengine/CMakeFiles/symengine.dir/fields.cpp.o] Error 1
CMakeFiles/Makefile2:162: recipe for target 'symengine/CMakeFiles/symengine.dir/all' failed
make[1]: *** [symengine/CMakeFiles/symengine.dir/all] Error 2
Makefile:127: recipe for target 'all' failed
make: *** [all] Error 2

I would like to update #1139 and not fiddle with the code-base any further. Please take a look, and suggest me any changes.

@ShikharJ ShikharJ changed the title [WIP] Additional Fixes Additional Fixes for #1139 Dec 5, 2016
@isuruf
Copy link
Copy Markdown
Member

isuruf commented Dec 5, 2016

Nice. It's good that down_cast figures out errors such as these. Can you push a branch with all the changes and I can have a look and fix these errors?

@certik
Copy link
Copy Markdown
Contributor

certik commented Dec 5, 2016

It looks like there are some const issues. One should be using a const_cast or something like that to cast away constness if needed.

ODictWrapper doesn't have virtual functions and hence not polymorphic
@ShikharJ
Copy link
Copy Markdown
Member Author

ShikharJ commented Dec 7, 2016

@isuruf If the work is complete, then we should get this merged.

@ShikharJ
Copy link
Copy Markdown
Member Author

ShikharJ commented Dec 9, 2016

ping @isuruf

@isuruf isuruf merged commit 114b85f into symengine:master Dec 9, 2016
@ShikharJ ShikharJ deleted the Issue1139 branch December 9, 2016 06:21
ranjithkumar007 pushed a commit to ranjithkumar007/symengine that referenced this pull request Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants