Skip to content

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.

4 participants