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

Fix trailing if element JMP lineno #11598

Closed
wants to merge 1 commit into from

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Jul 5, 2023

Having this lineno on the same last compiled element can lead to an incorrectly covered line number.

if (true) {
    if (false) {
        echo 'Never executed';
    }
} else {
}

The echo will be reported as covered because the JMP from the if (true) branch to the end of the else branch has the same lineno as the echo.

This is lacking a test because zend_dump.c does not have access to ctx->debug_level and I don't think it's worth adjusting all the cases.

Having this lineno on the same last compiled element can lead to an incorrectly
covered line number.

if (true) {
    if (false) {
        echo 'Never executed';
    }
} else {
}

The echo will be reported as covered because the JMP from the if (true) branch
to the end of the else branch has the same lineno as the echo.

This is lacking a test because zend_dump.c does not have access to
ctx->debug_level and I don't think it's worth adjusting all the cases.
@iluuu1994 iluuu1994 requested a review from derickr July 5, 2023 11:30
Copy link
Contributor

@derickr derickr left a comment

Choose a reason for hiding this comment

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

LGTM

@iluuu1994 iluuu1994 closed this in a5e89c5 Jul 5, 2023
@freiondrej-lokalise
Copy link

@iluuu1994 thank you so much for this! Most appreaciated 🙇

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.

None yet

3 participants