[API] Fix -Werror=alloc-size-larger-than= warning in runtime_context.h#3709
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3709 +/- ##
==========================================
- Coverage 89.95% 89.90% -0.05%
==========================================
Files 225 225
Lines 7273 7281 +8
==========================================
+ Hits 6542 6545 +3
- Misses 731 736 +5
🚀 New features to boost your workflow:
|
| } | ||
| if (size_ > max_capacity_) | ||
| { | ||
| return Context(); |
There was a problem hiding this comment.
Just curious - should it return the last valid context, instead of empty one ?
There was a problem hiding this comment.
Good question.
In my understanding, Scope will attach and detach contexts on creation and destruction, adding/removing from the stack.
If the code returns the last valid context, then we will have:
- every scope nested at depth max_capacity_, max_capacity_ + 1, ..., max_capacity_ + N share the same context.
- Scope at max_capacity_ + N will destroy the last valid scope on destruction, causing side effects.
So, I think Top() and Pop() should return an empty context instead, once the stack depth goes beyond the limit.
This is very theoretical, because any process with 1 million nested scopes in the call stack will never reach this point, being long dead, already killed for other reasons (stack overflow, out of memory).
lalitb
left a comment
lalitb
left a comment
There was a problem hiding this comment.
LGTM. The edge case shouldn’t occur in any normal usage scenario :)
| } | ||
| if (size_ > capacity_) | ||
| { | ||
| Resize(size_ * 2); |
There was a problem hiding this comment.
@marcalff
Did you see this line?
Resize(size_ * 2);
3 participants
Contributes to #2544
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes