Remove do{ } while(0) wrapper around error macros. #36011
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Could you please remove the |
As pointed out by Faless, a do{ } while(0) wrapper around a continue or
break just ends the do{ } while(0) loop. The do{ } while(0) loop exists
to enable the macro to be used as a function which requires a semicolon.
The alternative approach is to use an if(1) { } else ((void)0) wrapper.
Since the macro already has an if(unlikely(m_cond)) { } this patch simply
adds the else ((void)0) to this if statement instead.
For consistency all the macros have been updated in the same way, and
trailing else warnings corrected. However, the wrappers around ERR_PRINT
and WARN_PRINT were removed, because they generated too many ambiguous
trailing else warnings. They are also single line macros so a wrapper is
not needed.
|
@Faless I've dropped the |
|
Tested locally and it fixes a crash: #36028. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As pointed out by @Faless, a
do{ } while(0)wrapper around acontinueorbreakjust ends thedo{ } while(0)loop. Thedo{ } while(0)loop exists to enable the macro to be used as a function which requires a semicolon.The alternative approach is to use an
if(1) { } else ((void)0)wrapper. Since the macro already has anif(unlikely(m_cond)) { }this patch simply adds theelse ((void)0)to this if statement instead.For consistency all the macros have been updated in the same way, and trailing else warnings corrected. However, the wrappers around
ERR_PRINTandWARN_PRINTwere removed, because they generated too many ambiguous trailing else warnings. They are also single line macros so a wrapper is not needed.Fixes #33391 comment.
Bugsquad edit: Fixes #36028.