Account for deprecated GetPrototypeV2() and HolderV2() in v8 14.7#6470
Account for deprecated GetPrototypeV2() and HolderV2() in v8 14.7#6470
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Adds #if guards to replace deprecated V8 GetPrototypeV2() and HolderV2() with their renamed counterparts (GetPrototype() and Holder()) for V8 >= 14.7.
- [LOW]
#define HolderV2 Holderinresource.his never#undef'd. Since this is a header file, the macro leaks into every translation unit that includes it. Today this is harmless becauseHolderV2is only used withinresource.hitself, but it's poor hygiene and could cause surprising breakage if the token ever appears elsewhere. Adding an#undefafter the last use site would tighten the scope.
This review was generated by an AI assistant and may contain inaccuracies.
| // TODO(cleanup): Remove when unnecessary. | ||
| #if V8_MAJOR_VERSION >= 15 || (V8_MAJOR_VERSION == 14 && V8_MINOR_VERSION >= 7) | ||
| #define HolderV2 Holder | ||
| #endif |
There was a problem hiding this comment.
This macro is never #undef'd, so it leaks into every file that includes resource.h. Consider adding #undef HolderV2 at the end of the file (after the last use at line 1233) to prevent accidental token replacement elsewhere. The GetPrototypeV2 sites avoid this problem by using #if/#else blocks at each call site — you could do the same here for consistency, but an #undef at the bottom is the minimal fix.
| #endif | |
| #define HolderV2 Holder | |
| #endif // V8_MAJOR_VERSION >= 15 || (V8_MAJOR_VERSION == 14 && V8_MINOR_VERSION >= 7) |
Then at the end of the file (after line ~1233, or before the closing } // namespace):
#if V8_MAJOR_VERSION >= 15 || (V8_MAJOR_VERSION == 14 && V8_MINOR_VERSION >= 7)
#undef HolderV2
#endif|
I'm Bonk, and I've done a quick review of your PR. The changes add I posted one low-severity finding: the |
Relevant v8 change: https://chromium-review.googlesource.com/c/v8/v8/+/7624359