-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 crash when the expression exceed the depth #3429
fix crash when the expression exceed the depth #3429
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3429 +/- ##
==========================================
+ Coverage 85.23% 85.28% +0.05%
==========================================
Files 1277 1278 +1
Lines 119088 119303 +215
==========================================
+ Hits 101502 101747 +245
+ Misses 17586 17556 -30
Continue to review full report at Codecov.
|
And maybe not all expression will be checked by |
The And I didn't check non-recursive expressions in |
According to the Single Duty Principle, I think you should write a new visitor instead of do it in |
06bf960
to
f429029
Compare
Generally LGTM |
f429029
to
a1494eb
Compare
a1494eb
to
090a758
Compare
…bula into limit_depth_of_expression
checkDepth(); | ||
SCOPE_EXIT { recoverDepth(); }; | ||
if (!ok()) return; | ||
for (auto &arg : expr->args()->args()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider use any_of
void CheckDepthVisitor::visit(SubscriptExpression *expr) { | ||
checkDepth(); | ||
SCOPE_EXIT { recoverDepth(); }; | ||
if (!ok()) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider refactor to two function that return bool and use: fa() && fb();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this pr is closed
What type of PR is this?
What does this PR do?
fix crash When the expression's depth exceeds the maximum limit
(The depth is related to the stack size. This pr limited 815 is calculated based on the stack size of the development machine)
Which issue(s)/PR(s) this PR relates to?
fixed #3393
Special notes for your reviewer, ex. impact of this fix, etc:
Additional context:
Checklist:
Release notes:
Please confirm whether to reflect in release notes and how to describe: