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

internal check macros don't work #19503

Closed
targos opened this issue Mar 21, 2018 · 9 comments
Closed

internal check macros don't work #19503

targos opened this issue Mar 21, 2018 · 9 comments
Labels
confirmed-bug Issues with confirmed bugs.

Comments

@targos
Copy link
Member

targos commented Mar 21, 2018

This code:

CHECK_NE(addressType, undefined);

Is transformed to:

CHECK((addressType) !== (undefined));

This results in a runtime ReferenceError: CHECK is not defined. Macros should be expanded recursively.

/cc @devsnek

@devsnek
Copy link
Member

devsnek commented Mar 21, 2018

@targos interesting, I didn't observe that behaviour while testing. as it is the code is taken from v8's JS2C script so I don't know much about its specific workings. I can try to take a look later today.

@devsnek
Copy link
Member

devsnek commented Mar 23, 2018

@targos i can't reproduce this.

@BridgeAR
Copy link
Member

I just checked this and I am able to reproduce the problem.

CHECK(condition);
CHECK_EQ(condition, true);
DCHECK(condition);
DCHECK_EQ(condition, true);
// Becomes
do { if (!(condition)) (process._rawDebug("CHECK: condition == true"), process.abort()) } while (0);
CHECK((condition) === (true));
void(condition);
void(condition, true);

@BridgeAR BridgeAR added the confirmed-bug Issues with confirmed bugs. label Apr 12, 2018
@devsnek
Copy link
Member

devsnek commented Apr 12, 2018

i'm still unable to reproduce this 😕

@BridgeAR
Copy link
Member

How did you try to reproduce this so far?

@devsnek
Copy link
Member

devsnek commented Apr 12, 2018

I tried your inputs above but got the expected results out.

@targos
Copy link
Member Author

targos commented Apr 19, 2018

I still have this problem.

Can you try to reproduce with the following branch?
https://github.com/targos/node/tree/net-check-family

$ out/Release/node test/parallel/test-http-localaddress-bind-error.js
net.js:884
    CHECK((addressType) !== (undefined));
    ^

ReferenceError: CHECK is not defined
    at internalConnect (net.js:884:5)
    at defaultTriggerAsyncIdScope (internal/async_hooks.js:294:19)
    at GetAddrInfoReqWrap.emitLookup [as callback] (net.js:1080:9)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:55:10)

@jasnell
Copy link
Member

jasnell commented Aug 11, 2018

Ping @targos ... is this still an issue for you? I'm unable to reproduce.

@apapirovski
Copy link
Member

Given the lack of a follow up, I'm going to close this out. Please feel free to reopen if you believe I've made a mistake and this is still an existing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants