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

Hide compiler generated branches for try/catch blocks inside async state machine #716

Merged

Conversation

matteoerigozzi
Copy link
Contributor

@matteoerigozzi matteoerigozzi commented Jan 29, 2020

Fixes #465

Improve compiler generated branches detection inside MoveNext() method of async state machine.

This core remove all unexpected branches
image

@matteoerigozzi matteoerigozzi changed the title Hide compiler generated branches for try/catch blocks inside async state machine [WIP] Hide compiler generated branches for try/catch blocks inside async state machine Jan 30, 2020
@matteoerigozzi
Copy link
Contributor Author

Last update removes unexpected uncovered line after exception re-thrown inside catch block in the async state machine.
image

@matteoerigozzi matteoerigozzi changed the title [WIP] Hide compiler generated branches for try/catch blocks inside async state machine Hide compiler generated branches for try/catch blocks inside async state machine Jan 30, 2020
@MarcoRossignoli MarcoRossignoli added enhancement General enhancement request bug Something isn't working tenet-coverage Issue related to possible incorrect coverage and removed enhancement General enhancement request labels Feb 1, 2020
@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 1, 2020

The idea generally is ok to me, we need to be sure that if heuristics fail instrumentation go on, so some more check is not a great problem...I prefer worste perf than fail.
We need much more tests on it.

@MarcoRossignoli
Copy link
Collaborator

Please can you attach a full IL source of MoveNext to better review?I'd like to do another review round.

@matteoerigozzi matteoerigozzi changed the title Hide compiler generated branches for try/catch blocks inside async state machine [WIP]Hide compiler generated branches for try/catch blocks inside async state machine Feb 14, 2020
@matteoerigozzi
Copy link
Contributor Author

@MarcoRossignoli PTAL

I improved the heuristic logic and added several tests to validate it.

@matteoerigozzi matteoerigozzi changed the title [WIP]Hide compiler generated branches for try/catch blocks inside async state machine Hide compiler generated branches for try/catch blocks inside async state machine Feb 20, 2020
@matteoerigozzi
Copy link
Contributor Author

Attach generated IL for MoveNext() of this example (nested try/catch blocks with conditional exception rethrown)

image

MoveNext.txt

@MarcoRossignoli
Copy link
Collaborator

I'm checking...I need some time, this fix is "articulated", thank's

@MarcoRossignoli
Copy link
Collaborator

@matteoerigozzi can you rebase?

@matteoerigozzi
Copy link
Contributor Author

@MarcoRossignoli rebased!

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Mar 15, 2020

@matteoerigozzi have you tried to generate a report in release(some nop should disappear so could be good understand if all work as expected) and check if makes sense?

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

LGTM great job!

@MarcoRossignoli MarcoRossignoli merged commit 528956b into coverlet-coverage:master Mar 22, 2020
@matteoerigozzi matteoerigozzi deleted the matteo/exceptionrethrown branch April 7, 2020 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tenet-coverage Issue related to possible incorrect coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closing bracket uncovered in try-catch statement when exception is re-thrown in catch
2 participants