Rename GC*logueCallback to GCCallback for >4.0 #529
Conversation
| @@ -598,25 +598,33 @@ class TryCatch { | |||
| # define NAN_GC_CALLBACK(name) \ | |||
| void name(v8::Isolate *isolate, v8::GCType type, v8::GCCallbackFlags flags) | |||
|
|
|||
| #if NODE_MODULE_VERSION <= NODE_4_0_MODULE_VERSION | |||
| typedef v8::Isolate::GCEpilogueCallback GC_EPILOGUE_CALLBACK_TYPE; | |||
| typedef v8::Isolate::GCPrologueCallback GC_PROLOGUE_CALLBACK_TYPE; | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Thanks, @matthewloring. LGTM with one nit. |
Types were unified in https://codereview.chromium.org/1298113003 This change went into v8 4.6 which made it into node 5. The GC*logueCallback types have gone away completely in 4.9.
|
I was going with a different approach, but I guess this could work as a temporary fix as it does not change the external API. |
Right. I almost forgot about that one...
I'm OK with either one. This one offers a smoother transition while #489 might be a bit more future proof since it offers more ... wiggle room. "Might" because predictions are hard, specially if they concern the future. BTW, does anybody know an open source add-on that uses the GC*logue stuff? I'm kind of curious what people do with it. |
|
I'm not familiar with how people use this. I only encountered it when things crashed using nan with node 6.0-pre. I'm also happy with either solution, I was not aware of the other approach before starting this PR. |
|
I can merge this and put out a minor update if things don't work or give deprecation warnings in 5.0. Could even merge it regardless, if it happens to help for prereleases of 6. I have been meaning to get out the next minor for a couple of months already, but not gotten around to it, since nothing has been broken yet, AFAIK. The other PR is scheduled for NAN 3.0, which I intend to hold off until Node 6 in April or so. @agnat Yes, there is one, for which I originally wrote this functionality. I think it was this. I don't know of anything else that uses it. |
|
I've been experimenting with a branch of Node on top of v8 4.9 and this change is required to make nan work in this context. I believe it is also giving deprecation warnings in 5.0 based on the original v8 commit but I haven't confirmed this personally. It would definitely help me to get this fix in before nan 3.0. |
Rename GC*logueCallback to GCCallback for >4.0
Types were unified in https://codereview.chromium.org/1298113003
This change went into v8 4.6 which made it into Node 5. The
GC*logueCallback types have gone away completely in 4.9.