Clean up error_macros.h #33391
Clean up error_macros.h #33391
Conversation
madmiraal
requested review from
AndreaCatania,
bojidar-bg,
hpvb,
reduz,
vnen and
a team
as code owners
|
Submitted PR #33430 to address the build error raised by the Travis, Linux headless editor build. |
madmiraal
force-pushed
the
cleanup-error_macros
branch
3 times, most recently
from
9908e19 to
9560b56
Compare
|
This PR is now complete and ready for review. As stated in the initial PR message, The commits can be squashed before merging. They are kept separate to make it easier to review and fix conflicts if a rebase is necessary. Note: Since it is possible that a PR merged since this was rebased could use one of the removed macros, which would only be picked up by a CI test. It may be wise to rebase onto master again (as part of the final squash) before merging. |
madmiraal
force-pushed
the
cleanup-error_macros
branch
from
9560b56 to
29d3e39
Compare
|
Rebased following merge of #33517. |
madmiraal
force-pushed
the
cleanup-error_macros
branch
from
29d3e39 to
15d7673
Compare
|
Rebased following merge of #33595. |
madmiraal
force-pushed
the
cleanup-error_macros
branch
2 times, most recently
from
4a7fc63 to
3878a3b
Compare
|
Rebased following #33603. |
madmiraal
force-pushed
the
cleanup-error_macros
branch
from
3878a3b to
dfc6bbc
Compare
madmiraal
force-pushed
the
cleanup-error_macros
branch
2 times, most recently
from
d74f478 to
11286f8
Compare
- Add do..while(0) wrapper to ERR_FAIL_NULL macros. - Add do..while(0) wrapper to ERR_FAIL_COND macros. - Add do..while(0) wrapper to ERR_CONTINUE macros. - Add do..while(0) wrapper to ERR_BREAK macros. - Add do..while(0) wrapper to CRASH_COND macros. - Add do..while(0) wrapper to ERR_FAIL macros. - Add do..while(0) wrapper to ERR_PRINT macros. - Add do..while(0) wrapper to WARN_PRINT macros. - Add do..while(0) wrapper to WARN_DEPRECATED macros. - Add do..while(0) wrapper to CRASH_NOW macros.
- Remove trailing semicolons from ERR_FAIL_INDEX macros. - Remove trailing semicolons from ERR_FAIL_UNSIGNED_INDEX macros. - Remove trailing semicolons from CRASH_BAD_INDEX macros. - Remove trailing semicolons from CRASH_BAD_UNSIGNED_INDEX macros.
madmiraal
force-pushed
the
cleanup-error_macros
branch
from
7bbe6f1 to
6776046
Compare
There was a problem hiding this comment.
Changes look great.
Thanks for going through the tedious work of rebasing it periodically and keep well-scoped commits. Should be good to merge now, I'll just have to handle the build issues due to ERR_PRINTS removal when rebasing vulkan, but that's business as usual :D
|
Thanks! |
akien-mga
removed request for
a team,
hpvb,
bojidar-bg,
vnen,
neikeq and
AndreaCatania
Remove the last ERR_PRINTS that was missed by #33391
|
|
| if (unlikely(m_cond)) { \ | ||
| _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "Condition \"" _STR(m_cond) "\" is true. Continuing."); \ | ||
| continue; \ | ||
| } \ | ||
| } | ||
| } while (0) |
There was a problem hiding this comment.
continue will end inner loop, but not the outer one (same below in ERR_CONTINUE_MSG, same in breaks ERR_BREAK and ERR_BREAK_MSG).
There was a problem hiding this comment.
Good point. Shall I submit a PR to wrap these in if(1) { } else instead?
EDIT: Actually it's easier to just add a dangling else to the existing if (unlikely(m_cond))
There was a problem hiding this comment.
EDIT: Actually it's easier to just add a dangling else to the existing if (unlikely(m_cond))
But code such as this would still be valid then?
ERR_CONTINUE(x != 0) // No semicolon
z()Maybe add a trailing do {} while(0)?
There was a problem hiding this comment.
But code such as this would still be valid then?
Actually the compiler warns about the dangling else; so it probably needs to be if(unlikely(m_cond)) { } else ((void)0)
This pull request aims to achieve a number of things:
To make it easier to review and rebase when necessary, the changes are added as separate commits. They can be squashed before merging.
Note: Although the initial commit makes significant changes to error_macros.h, these are limited to code ordering and comments to make it consistent and more readable. It has no impact on any code that uses error_macros.h. This initial commit makes it easier to review the following commits.