Skip to content

#12158: improve check: variableScope is not reported when there is el…#5758

Merged
chrchr-github merged 2 commits into
cppcheck-opensource:mainfrom
olabetskyi:ticket_12158
Dec 24, 2023
Merged

#12158: improve check: variableScope is not reported when there is el…#5758
chrchr-github merged 2 commits into
cppcheck-opensource:mainfrom
olabetskyi:ticket_12158

Conversation

@olabetskyi
Copy link
Copy Markdown
Collaborator

variableScope is not reported when there is else if

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@danmar danmar added the merge-after-next-release Wait with merging this PR until after the next Release label Dec 13, 2023
Comment thread test/testother.cpp Outdated
" if ( value > 100U ) { }\n"
" else if( value > 50U ) { }\n"
" else{\n"
" for( i = 0U; i < 5U; i++ ) {}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olabetskyi nothing is wrong here but please add newline..

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olabetskyi I assume no warning will be written for the code below.. feel free to investigate how we can warn for this code:

void f( uint32_t value ) {
    uint32_t i = 0U;
    if ( value > 100U ) { }
    else {
        if( value > 50U ) { }
        else {
            for( i = 0U; i < 5U; i++ ) {}
        }
    }
}

@olabetskyi
Copy link
Copy Markdown
Collaborator Author

@danmar I've added your example to tests and it was caught as expected. Bacuse for the provided solution there is no difference between else if and else { if

@chrchr-github chrchr-github merged commit e553940 into cppcheck-opensource:main Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-after-next-release Wait with merging this PR until after the next Release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants