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.

Sign up for GitHub

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

Jump to bottom

[NFC]Do not use recursion for CounterMappingContext::evaluate #66961

Merged
merged 1 commit into from
Sep 22, 2023
Merged

[NFC]Do not use recursion for CounterMappingContext::evaluate #66961

merged 1 commit into from
Sep 22, 2023

Conversation

shen3qing1
Copy link
Contributor

@shen3qing1 shen3qing1 commented Sep 20, 2023 •
edited

This causes stack overflows for real-world coverage reports.

Ran $ build/bin/llvm-lit -a llvm/test/tools/llvm-cov locally and passed.

[NFC]Do not use recursion for CounterMappingContext::evaluate
4b65694 
This causes stack overflows for real-world coverage reports.
@shen3qing1
Copy link
Contributor Author

@jroelofs @MaskRay for reviewers. (Somehow I cannot add the reviewers on the page, lack permissions or something.
5gqULus9xsyQTex
)

jroelofs
jroelofs approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@jroelofs jroelofs left a comment

Choose a reason for hiding this comment

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

LGTM

@shen3qing1
Copy link
Contributor Author

Hey @dtellenbach @MaskRay do you mind taking a look too? Thanks!

@jroelofs
Copy link
Contributor

@shen3qing1 let's merge this. Addressing issues post-commit is fine, if they have any comments.

@jroelofs jroelofs merged commit 487c784 into llvm:main Sep 22, 2023
2 checks passed
@shen3qing1
Copy link
Contributor Author

Thanks, @jroelofs! :)

@kstoimenov kstoimenov mentioned this pull request Sep 22, 2023
Closed
@MaskRay
Copy link
Member

MaskRay commented Sep 24, 2023

LGTM. I still haven't had a good way to track email spam after github pull request transition...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jroelofs jroelofs jroelofs approved these changes

@MaskRay MaskRay Awaiting requested review from MaskRay

@dtellenbach dtellenbach Awaiting requested review from dtellenbach

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@shen3qing1 @jroelofs @MaskRay