Skip to content
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

Create a utility function that wraps assert but instead throws #29

Closed
piraka9011 opened this issue Jan 13, 2020 · 14 comments
Closed

Create a utility function that wraps assert but instead throws #29

piraka9011 opened this issue Jan 13, 2020 · 14 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@piraka9011
Copy link

Inspired by Kotlin's Assert, it would be helpful to have a wrapper around assert that instead throws to provide a meaningful stack trace.

@emersonknapp emersonknapp added the enhancement New feature or request label Jan 13, 2020
@emersonknapp
Copy link
Contributor

Do you mean a wrapper around assert? Or do you mean an assert-like functionality that works in non-debug builds?

@zmichaels11
Copy link
Contributor

I think it would be better as assert-like

@tfoote
Copy link
Contributor

tfoote commented Jan 14, 2020

Please consider picking up ros2/rcutils#112 that got deprioritized previously.

@zmichaels11
Copy link
Contributor

Possible implementation: #31

@wjwwood
Copy link
Member

wjwwood commented Jan 23, 2020

What is the benefit of having an assert that throws over the standard one that uses abort? Doesn't gdb break on abort just like it does with an exception? Or were you thinking of catching the exceptions? I'm just trying to understand the motivation.

I do understand a require or check that always asserts even when in release mode.

@wjwwood
Copy link
Member

wjwwood commented Jan 23, 2020

Please consider picking up ros2/rcutils#112 that got deprioritized previously.

This could be a good solution, but note that this implementation is a bit different, as in the case that the condition fails it logs to the rcutils logger (which may or may not be desired) and it uses a "DebugBreak" which is a trap, rather than throwing an exception (which it cannot do since it might be used in C only code) and rather than using abort() which on Linux would result in a SIGABRT (I think).

If the main goal is to have an assert() that works in release mode too (a.k.a. check or required) then ros2/rcutils#112 might be overkill.

@zmichaels11
Copy link
Contributor

A benefit of throwing vs abort is that failure can be checked in unit tests.

@wjwwood
Copy link
Member

wjwwood commented Jan 23, 2020

I see, Google Test has "Death tests" which work with asserts but I guess they're hard to use and sometimes don't work as expected on Windows:

https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#how-to-write-a-death-test

@zmichaels11
Copy link
Contributor

Also, an exception carries the additional information that the code crashed due to a check, require, or assert

@wjwwood
Copy link
Member

wjwwood commented Jan 23, 2020

Also, an exception carries the additional information that the code crashed due to a check, require, or assert

You mean in gdb? You should be able to see the same kind of information as with an unhandled exception, but I haven't compared the two closely.

The "debugbreak" style proposed in ros2/rcutils#112 might be preferable since it can be called from and across pure C99 layers too. Though I guess those cannot be as easily caught in tests as the exceptions.

@zmichaels11
Copy link
Contributor

No, I meant in tests.

@wjwwood
Copy link
Member

wjwwood commented Jan 23, 2020

I see what you mean.

@zmichaels11
Copy link
Contributor

I'm thinking it can be improved by introducing an AssertionException that way in tests, you can EXPECT_THROWS on the AssertionException and know that your code is properly failing

@ivanpauno ivanpauno added the help wanted Extra attention is needed label Feb 6, 2020
@emersonknapp
Copy link
Contributor

Issue closed in #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants