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 halide_debug_assert() macro (#6384) #6385

Closed
wants to merge 9 commits into from

Conversation

steven-johnson
Copy link
Contributor

@steven-johnson steven-johnson commented Nov 3, 2021

Also convert usage of halide_assert()/HALIDE_CHECK() in hashmap.h and gpu_context_common.h to halide_debug_assert(), as all the usages looked to be appropriate for debug-mode only.

Also convert usage of `halide_assert()`/`HALIDE_CHECK()` in hashmap.h and gpu_context_common.h to `HALIDE_DEBUG_ASSERT()`, as all the usages looked to be appropriate for debug-mode only.
@zvookin
Copy link
Member

zvookin commented Nov 3, 2021

Do we really need to litter the code long all caps names? We should use lots of asserts and checks and this approach is just horrible from a readability point of view. It draws massive attention to the name of the macro, which is just not at all interesting. It's like if every if statement was written "HEY_IM_AN_IF_STATEMENT_PAY_ATTENTION_TO_ME".

@steven-johnson
Copy link
Contributor Author

Do we really need to litter the code long all caps names?

For HALIDE_CHECK I think that SCREAMING_SNAKE_CASE is appropriate because it really should jump out at you. For debug-assert, though, probably not. would halide_debug_assert() be a better choice?

@steven-johnson steven-johnson changed the title Add HALIDE_DEBUG_ASSERT() macro (#6384) Add halide_debug_assert() macro (#6384) Nov 3, 2021
@steven-johnson
Copy link
Contributor Author

(This needs #6382 to land first)

steven-johnson added a commit that referenced this pull request Nov 4, 2021
Also convert usage of halide_assert()/HALIDE_CHECK() in hashmap.h and gpu_context_common.h to halide_debug_assert(), as all the usages looked to be appropriate for debug-mode only.

(Rebased version of #6385, which this replaces)
@steven-johnson
Copy link
Contributor Author

Closing in favor of identical-but-rebased #6390

@steven-johnson steven-johnson deleted the srj/halide-debug-assert branch November 4, 2021 17:52
steven-johnson added a commit that referenced this pull request Nov 8, 2021
* Add halide_debug_assert() macro

Also convert usage of halide_assert()/HALIDE_CHECK() in hashmap.h and gpu_context_common.h to halide_debug_assert(), as all the usages looked to be appropriate for debug-mode only.

(Rebased version of #6385, which this replaces)

* appease clang-format
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