cppgc: Short-circuit callback checking in GCInfo [v8/v8 : main]
0 views
Skip to first unread message
Anton Bikineev (Gerrit)
Jun 4, 2024, 9:39:35 AMJun 4
to Michael Lippautz, V8 LUCI CQ, cbruni...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Anton Bikineev voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Jun 4, 2024, 9:43:51 AMJun 4
to Anton Bikineev, V8 LUCI CQ, cbruni...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Anton Bikineev
Michael Lippautz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
2 comments
Patchset-level comments
File include/cppgc/internal/gc-info.h
Line 135, Patchset 1 (Latest):
GCInfoTrait<T>::CheckCallbacksAreDefined() &&Michael Lippautz . unresolved
Do these still need to return true? I guess we can just call these methods with the same result?
Related details
Attention is currently required from:
- Anton Bikineev
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
Jun 4, 2024, 11:03:00 AMJun 4
to Michael Lippautz, V8 LUCI CQ, cbruni...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Anton Bikineev added 1 comment![]( "Open in Gerrit")
File include/cppgc/internal/gc-info.h
Line 135, Patchset 1 (Latest):
GCInfoTrait<T>::CheckCallbacksAreDefined() &&Michael Lippautz . unresolved
Do these still need to return true? I guess we can just call these methods with the same result?
Anton Bikineev
true, it doesn't have to return anything. `if constexpr` serves that the functions are not instantiated for types that don't satisfy the condition.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
Jun 4, 2024, 11:04:58 AMJun 4
to Michael Lippautz, V8 LUCI CQ, cbruni...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Anton Bikineev voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Jun 4, 2024, 5:45:03 PMJun 4
to Anton Bikineev, Michael Lippautz, cbruni...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: include/cppgc/internal/gc-info.h
Insertions: 3, Deletions: 5.
@@ -94,12 +94,11 @@
return index;
}
- static constexpr bool CheckCallbacksAreDefined() {
+ static constexpr void CheckCallbacksAreDefined() {
// No USE() macro available.
(void)static_cast<TraceCallback>(TraceTrait<T>::Trace);
(void)static_cast<FinalizationCallback>(FinalizerTrait<T>::kCallback);
(void)static_cast<NameCallback>(NameTrait<T>::GetName);
- return true;
}
};
@@ -132,9 +131,8 @@
kBothTypesAreTriviallyDestructible ||
kHasCustomFinalizerDispatchAtBase) &&
!kWantsDetailedObjectNames) {
- GCInfoTrait<T>::CheckCallbacksAreDefined() &&
- GCInfoTrait<
- ParentMostGarbageCollectedType>::CheckCallbacksAreDefined();
+ GCInfoTrait<T>::CheckCallbacksAreDefined();
+ GCInfoTrait<ParentMostGarbageCollectedType>::CheckCallbacksAreDefined();
return true;
}
return false;
```
Change information
Commit message:
cppgc: Short-circuit callback checking in GCInfo
If gc-info for child is unfolded, we don't actually have to try to
call the parent's dtor, which can be e.g. protected.
Change-Id: I41e263d29fdeb1cbcb65bb066c505bee4b516ad1
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Commit-Queue: Anton Bikineev <biki...@chromium.org>
Auto-Submit: Anton Bikineev <biki...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94247}
Files:
- M include/cppgc/internal/gc-info.h
Change size: S
Delta: 1 file changed, 12 insertions(+), 10 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages