Skip to content

Conversation

erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Aug 8, 2023

This seems to fix #13752
(Someone else confirm that?)

Other languages already have a similar limitation in how they track regular expressions.

An evaluation looks slightly weird, but I think it's good.

@github-actions github-actions bot added the Java label Aug 8, 2023
@erik-krogh erik-krogh marked this pull request as ready for review August 9, 2023 07:25
@erik-krogh erik-krogh requested a review from a team as a code owner August 9, 2023 07:25
@erik-krogh erik-krogh added the no-change-note-required This PR does not need a change note label Aug 9, 2023
@atorralba
Copy link
Contributor

This should be already fixed by #13987. Nonetheless, since you mentioned that other languages already apply this limitation, should we also add it as an additional precaution?

@erik-krogh
Copy link
Contributor Author

This should be already fixed by #13987.

No, it's not.
#13987 only fixed a join order that became visible after this PR made the analysis finish.

I've been running java/redos for half an hour on my laptop (M1 Max), and it's not looking like it's going to finish.
So we still need this.

@erik-krogh erik-krogh merged commit 08ef31d into github:main Aug 18, 2023
@aschackmull
Copy link
Contributor

It's a bit sad that we have to restrict flow so much in this way. The fieldFlowBranchLimit is a rather blunt tool that's certain to introduce more FNs, so it would be nice to eventually improve on the underlying reasons for the poor performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants