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

Add assert to p functions #110

Closed
wants to merge 1 commit into from
Closed

Conversation

ycho
Copy link
Collaborator

@ycho ycho commented Mar 2, 2018

I am not so sure we need assert functions with Rust as well.
Because I see that Rust program raises flag
if index value larger than defined is ever used during runtime.
With RUST_BACKTRACE=1, debugger also shows the call stack for back trace.

I hope there is other benefit with assert, which I don't know at the moment.

I am not so sure we need assert functions with Rust as well.
Because I see that Rust program raises flag
if index value larger than defined is ever used during runtime.
With RUST_BACKTRACE=1, debugger also shows the call stack for back trace.

I hope there is other benefit with assert, which I don't know at the moment.
@lu-zero
Copy link
Collaborator

lu-zero commented Mar 2, 2018

Asserts in rust are mainly useful for refactoring/documentation and (often placebo) compiler hints. I wouldn't use them here.

@ycho
Copy link
Collaborator Author

ycho commented Mar 2, 2018

In fact, in my rust dev env, even assert is not a minimum tool for debugging crash point and seeing call stack. Once assert is met, my IDE (Eclipse) does not show any call stack. So I always adds extra if { // with breakpoint; } to see the call stack. The RUST_BACKTRACE=1 does not work to show call stack in Eclipse as well, though it shows in command line.
I believe you have dev env which can see call stack when rust program crash, w/o any extra caveat.
I assume rust support for developers with IDE is not mature, so I believe having assert will help those folks including me.

@tdaede
Copy link
Collaborator

tdaede commented Mar 2, 2018

I think the issue is rust-lang/rust#21102

In particular, it seems like you just need to set a breakpoint on rust_panic, then you will see the traces in your debugger.

@lu-zero
Copy link
Collaborator

lu-zero commented Mar 3, 2018

@ycho alternatively -C panic=abort would make the panic a normal abort.

@ycho
Copy link
Collaborator Author

ycho commented Mar 3, 2018

I see, that is the info TD-Linux was seeing after he has instructed how to possibly live w/o asserts in rust program in Eclipse or VSCode live, both were successful. He managed to use enter the command "b rust_panic" in debug command window inside IDE, as he suggested in above reply.
So... yes, lu-zero, I think I can go ahead w/o asserts.
(And I'd ask what about other pre-existing asserts? Which are not from me...)

@ycho ycho closed this Mar 3, 2018
@lu-zero
Copy link
Collaborator

lu-zero commented Mar 3, 2018

A good deal of them are either in the test code or used to document the functions input ranges in case we want to change the input types that I can see. We could switch the latter to debug_assert!.

@ycho
Copy link
Collaborator Author

ycho commented Mar 4, 2018

@lu-zero btw, where should I put "-C panic=abort"? Tried put it at the end of %cargo build --bin rav1e, which did not work.
Also tried adding below to Cargo.toml:
[profile.denug]
panic = "abort"

that did not work as well.

@ycho ycho deleted the add_assert_p_func branch March 13, 2018 21:45
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