Skip to content
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

Nogcmacro #489

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Nogcmacro #489

wants to merge 2 commits into from

Conversation

kkoopa
Copy link
Collaborator

@kkoopa kkoopa commented Oct 10, 2015

Tried to remove the dependence on macros.
The gc callback thing feels a bit clumsy.

nan.h Outdated
}
}

# define NAN_MODULE(modname, regfunc) \

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@agnat
Copy link
Collaborator

agnat commented Oct 10, 2015

The gc callback thing feels a bit clumsy.

I see you pulled the direct call trick ... other than that. Please elaborate...

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2015

I would like to get the callback as a function argument, like in all other parts of the API, but that is not possible. This will remain a slightly odd case.

On October 10, 2015 1:13:19 PM GMT+03:00, David Siegel notifications@github.com wrote:

The gc callback thing feels a bit clumsy.

I see you pulled the direct call trick ... other than that. Please
elaborate...


Reply to this email directly or view it on GitHub:
#489 (comment)

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2015

Yeah, this is all mostly for the next major, whenever that is. Not worth breaking compatibility over these small things alone. Just figured I'd explore the possibilities. This seems to be the best way of defining callbacks when no arbitrary data arguments are available.

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2015

The ugliness could be avoided by using a static map of gc callbacks, but I don't know if it is worth it.

@agnat
Copy link
Collaborator

agnat commented Oct 10, 2015

Hm, yeah... a homegrown v-table... not sure... in my book that would be even more ugly as it introduces a call overhead of O(n log(n)) (I think) with n being the number of NAN based modules. Not that anyone would notice but I like the template version better...

@agnat
Copy link
Collaborator

agnat commented Oct 10, 2015

BTW, I don't think it's ugly... it's merely a bit inconsistent. Actually I think I like it. The pattern has interesting properties.

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2015

This would only be for the gc callbacks. There n would grow with the number of gc callbacks. How likely are you to use many of such an obscure item? Inconsistency is arguably ugliness, but I prefer this over macros. C++11 lifts a lot of these restrictions, but we can't use that everywhere yet, which is kind of limiting.

@kkoopa kkoopa added this to the 3.0 milestone Oct 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants