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

Catch Switch Added #815

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@CodeMaxx
Copy link
Contributor

CodeMaxx commented Feb 15, 2016

Addresses #812
@isuruf

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 15, 2016

I added HAVE_SYMENGINE_CATCH though it is not being used as of now but might be used in future.

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 15, 2016

@CodeMaxx, what I meant was that Catch is catching all exceptions with catch(...) statements. CMake switch was to disable catching of all exceptions

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 15, 2016

@isuruf Oh I see... so what all files contain these catch statements? Do I need to disable them in each file or can I disable them from the catch.cpp file?

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 15, 2016

Currently I found catch statements in Teuchos_TestForException.hpp, nonius.h++ Teuchos_RCPNode.hpp

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 15, 2016

Are you looking for somthing like this? (Teuchos_TestForException.hpp)

#ifdef HAVE_SYMENGINE_CATCH
    #define TEUCHOS_TEST_FOR_EXCEPTION_PRINT(throw_exception_test, Exception, msg, out_ptr) \
    try { \
      TEUCHOS_TEST_FOR_EXCEPTION(throw_exception_test, Exception, msg); \
    } \
    catch(const std::exception &except) { \
      std::ostream *l_out_ptr = (out_ptr); \
      if(l_out_ptr) { \
        *l_out_ptr \
          << "\nThrowing an std::exception of type \'"<<Teuchos::typeName(except) \
          <<"\' with the error message: " \
          << except.what(); \
      } \
      throw; \
    }
#endif
@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 16, 2016

@isuruf Ping

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 16, 2016

I was referring to the catch statements in catch.hpp. #defines is the way to go. Something like SYMENGINE_DISABLE_CATCH_ALL

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 18, 2016

@isuruf So you mean the developer manually changes the value of SYMENGINE_DISABLE_CATCH_ALL in the catch.hpp to true when he wants to get stacktraces from teuchos ?

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 18, 2016

No, like you did here, a CMake option would be nice to have.

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 18, 2016

@isuruf Do I need to skip all the try-catch blocks? Even something like this ?

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 18, 2016

Also I can't figure out how to use a variable declared in the CMakeList files inside catch.hpp


Ok so I figured this out.

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 26, 2016

@isuruf Since all test use catch syntax, how would disabling try catch blocks cause stacktraces to show up? When I placed ALL try catch blocks inside an if statement or ifdef .... the tests passed even though I did some manipulation in the test file so as it fails and I get a stacktrace.
I'm pushing the code so you can review.

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:catch branch from 16218f4 to 9e220c1 Feb 26, 2016

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 26, 2016

Disabling all try statements will not throw an exception at all. What I meant was that when an exception happens, Catch will catch the exception and report it. If we disable this catch all, then the statement inside try will execute and if there is an exception then it will not be handled. If an exception is unhandled program is aborted and teuchos will print the stacktrace.

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 26, 2016

Yeah had figured that out ... thanks anways 😄

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 26, 2016

@isuruf I'm not sure if this works.Please Review

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:catch branch from f46a1a1 to ec28c85 Feb 26, 2016

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 26, 2016

I don't think it does. I'll look into this when I get some time.

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Mar 9, 2016

@isuruf Anything on this?

@CodeMaxx CodeMaxx closed this Apr 23, 2016

@CodeMaxx CodeMaxx reopened this Apr 26, 2016

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