From 12f48efc2a9eb8a0eec2491bd2f223dfc1b1ca15 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 17:00:34 +0000 Subject: [PATCH 01/55] N-API port initial implementation (WIP) --- binding.gyp | 2 +- package-lock.json | 360 +++++++++++++-------------- package.json | 4 +- src/module.cpp | 15 +- src/module_utils.hpp | 37 +-- src/object_async/hello_async.cpp | 298 ++++++++-------------- src/object_async/hello_async.hpp | 24 +- src/object_sync/hello.cpp | 167 +++---------- src/object_sync/hello.hpp | 28 +-- src/standalone/hello.cpp | 8 +- src/standalone/hello.hpp | 9 +- src/standalone_async/hello_async.cpp | 106 ++++---- src/standalone_async/hello_async.hpp | 8 +- test/hello_object.test.js | 12 +- test/hello_object_async.test.js | 8 +- 15 files changed, 440 insertions(+), 646 deletions(-) diff --git a/binding.gyp b/binding.gyp index 7c3dbe5..7460e14 100644 --- a/binding.gyp +++ b/binding.gyp @@ -15,7 +15,7 @@ # It's a variable to make easy to pass to # cflags (linux) and xcode (mac) 'system_includes': [ - "-isystem <(module_root_dir)/= 1.4 < 2" } }, "extsprintf": { @@ -316,7 +316,7 @@ "integrity": "sha1-LEBFC5NI6X8oEyJZO6lnBLmr1NQ=", "dev": true, "requires": { - "is-function": "1.0.1" + "is-function": "~1.0.0" } }, "foreach": { @@ -335,9 +335,9 @@ "resolved": "https://registry.npmjs.org/form-data/-/form-data-2.3.2.tgz", "integrity": "sha1-SXBJi+YEwgwAXU9cI67NIda0kJk=", "requires": { - "asynckit": "0.4.0", + "asynckit": "^0.4.0", "combined-stream": "1.0.6", - "mime-types": "2.1.18" + "mime-types": "^2.1.12" } }, "fs-extra": { @@ -345,9 +345,9 @@ "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz", "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==", "requires": { - "graceful-fs": "4.1.11", - "jsonfile": "4.0.0", - "universalify": "0.1.1" + "graceful-fs": "^4.1.2", + "jsonfile": "^4.0.0", + "universalify": "^0.1.0" } }, "fs-minipass": { @@ -355,7 +355,7 @@ "resolved": "https://registry.npmjs.org/fs-minipass/-/fs-minipass-1.2.5.tgz", "integrity": "sha512-JhBl0skXjUPCFH7x6x61gQxrKyXsxB5gcgePLZCwfyCGGsTISMoIeObbrvVeP6Xmyaudw4TT43qV2Gz+iyd2oQ==", "requires": { - "minipass": "2.3.3" + "minipass": "^2.2.1" } }, "fs.realpath": { @@ -374,14 +374,14 @@ "resolved": "https://registry.npmjs.org/gauge/-/gauge-2.7.4.tgz", "integrity": "sha1-LANAXHU4w51+s3sxcCLjJfsBi/c=", "requires": { - "aproba": "1.2.0", - "console-control-strings": "1.1.0", - "has-unicode": "2.0.1", - "object-assign": "4.1.1", - "signal-exit": "3.0.2", - "string-width": "1.0.2", - "strip-ansi": "3.0.1", - "wide-align": "1.1.2" + "aproba": "^1.0.3", + "console-control-strings": "^1.0.0", + "has-unicode": "^2.0.0", + "object-assign": "^4.1.0", + "signal-exit": "^3.0.0", + "string-width": "^1.0.1", + "strip-ansi": "^3.0.1", + "wide-align": "^1.1.0" } }, "getpass": { @@ -389,7 +389,7 @@ "resolved": "https://registry.npmjs.org/getpass/-/getpass-0.1.7.tgz", "integrity": "sha1-Xv+OPmhNVprkyysSgmBOi6YhSfo=", "requires": { - "assert-plus": "1.0.0" + "assert-plus": "^1.0.0" } }, "glob": { @@ -397,12 +397,12 @@ "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.2.tgz", "integrity": "sha512-MJTUg1kjuLeQCJ+ccE4Vpa6kKVXkPYJ2mOCQyUuKLcLQsdrMCpBPUi8qVE6+YuaJkozeA9NusTAw3hLr8Xe5EQ==", "requires": { - "fs.realpath": "1.0.0", - "inflight": "1.0.6", - "inherits": "2.0.3", - "minimatch": "3.0.4", - "once": "1.4.0", - "path-is-absolute": "1.0.1" + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.0.4", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" } }, "graceful-fs": { @@ -420,8 +420,8 @@ "resolved": "https://registry.npmjs.org/har-validator/-/har-validator-5.0.3.tgz", "integrity": "sha1-ukAsJmGU8VlW7xXg/PJCmT9qff0=", "requires": { - "ajv": "5.5.2", - "har-schema": "2.0.0" + "ajv": "^5.1.0", + "har-schema": "^2.0.0" } }, "has": { @@ -430,7 +430,7 @@ "integrity": "sha1-hGFzP1OLCDfJNh45qauelwTcLyg=", "dev": true, "requires": { - "function-bind": "1.1.1" + "function-bind": "^1.0.2" } }, "has-unicode": { @@ -443,9 +443,9 @@ "resolved": "https://registry.npmjs.org/http-signature/-/http-signature-1.2.0.tgz", "integrity": "sha1-muzZJRFHcvPZW2WmCruPfBj7rOE=", "requires": { - "assert-plus": "1.0.0", - "jsprim": "1.4.1", - "sshpk": "1.14.1" + "assert-plus": "^1.0.0", + "jsprim": "^1.2.2", + "sshpk": "^1.7.0" } }, "iconv-lite": { @@ -453,7 +453,7 @@ "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.23.tgz", "integrity": "sha512-neyTUVFtahjf0mB3dZT77u+8O0QB89jFdnBkd5P1JgYPbPaia3gXXOVL2fq8VyU2gMMD7SaN7QukTB/pmXYvDA==", "requires": { - "safer-buffer": "2.1.2" + "safer-buffer": ">= 2.1.2 < 3" } }, "ieee754": { @@ -467,7 +467,7 @@ "resolved": "https://registry.npmjs.org/ignore-walk/-/ignore-walk-3.0.1.tgz", "integrity": "sha512-DTVlMx3IYPe0/JJcYP7Gxg7ttZZu3IInhuEhbchuqneY9wWe5Ojy2mXLBaQFUQmo0AW2r3qG7m1mg86js+gnlQ==", "requires": { - "minimatch": "3.0.4" + "minimatch": "^3.0.4" } }, "inflight": { @@ -475,8 +475,8 @@ "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", "integrity": "sha1-Sb1jMdfQLQwJvJEKEHW6gWW1bfk=", "requires": { - "once": "1.4.0", - "wrappy": "1.0.2" + "once": "^1.3.0", + "wrappy": "1" } }, "inherits": { @@ -506,7 +506,7 @@ "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz", "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", "requires": { - "number-is-nan": "1.0.1" + "number-is-nan": "^1.0.0" } }, "is-function": { @@ -521,7 +521,7 @@ "integrity": "sha1-VRdIm1RwkbCTDglWVM7SXul+lJE=", "dev": true, "requires": { - "has": "1.0.1" + "has": "^1.0.1" } }, "is-symbol": { @@ -577,7 +577,7 @@ "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", "requires": { - "graceful-fs": "4.1.11" + "graceful-fs": "^4.1.6" } }, "jsprim": { @@ -602,15 +602,15 @@ "resolved": "https://registry.npmjs.org/mason-js-sdk/-/mason-js-sdk-0.1.4.tgz", "integrity": "sha1-vrC0YA4VBCT68uuUcEXl1YtweSE=", "requires": { - "d3-queue": "3.0.7", + "d3-queue": "^3.0.7", "extfs": "0.0.7", - "fs-extra": "4.0.3", - "minimist": "1.2.0", - "mkdirp": "0.5.1", - "needle": "2.2.1", - "npmlog": "4.1.2", - "request": "2.87.0", - "tar": "4.4.4" + "fs-extra": "^4.0.2", + "minimist": "^1.2.0", + "mkdirp": "^0.5.1", + "needle": "^2.2.0", + "npmlog": "^4.1.2", + "request": "^2.83.0", + "tar": "^4.0.2" } }, "mime-db": { @@ -623,7 +623,7 @@ "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.18.tgz", "integrity": "sha512-lc/aahn+t4/SWV/qcmumYjymLsWfN3ELhpmVuUFjgsORruuZPVSwAQryq+HHGvO/SI2KVX26bx+En+zhM8g8hQ==", "requires": { - "mime-db": "1.33.0" + "mime-db": "~1.33.0" } }, "minimatch": { @@ -631,7 +631,7 @@ "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", "requires": { - "brace-expansion": "1.1.11" + "brace-expansion": "^1.1.7" } }, "minimist": { @@ -644,8 +644,8 @@ "resolved": "https://registry.npmjs.org/minipass/-/minipass-2.3.3.tgz", "integrity": "sha512-/jAn9/tEX4gnpyRATxgHEOV6xbcyxgT7iUnxo9Y3+OB0zX00TgKIv/2FZCf5brBbICcwbLqVv2ImjvWWrQMSYw==", "requires": { - "safe-buffer": "5.1.2", - "yallist": "3.0.2" + "safe-buffer": "^5.1.2", + "yallist": "^3.0.0" }, "dependencies": { "safe-buffer": { @@ -660,7 +660,7 @@ "resolved": "https://registry.npmjs.org/minizlib/-/minizlib-1.1.0.tgz", "integrity": "sha512-4T6Ur/GctZ27nHfpt9THOdRZNgyJ9FZchYO1ceg5S8Q3DNLCKYy44nCZzgCJgcvx2UM8czmqak5BCxJMrq37lA==", "requires": { - "minipass": "2.3.3" + "minipass": "^2.2.1" } }, "mkdirp": { @@ -683,36 +683,36 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=" }, - "nan": { - "version": "2.10.0", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.10.0.tgz", - "integrity": "sha512-bAdJv7fBLhWC+/Bls0Oza+mvTaNQtP+1RyhhhvD95pgUJz6XM5IzgmxOkItJ9tkoCiplvAnXI1tNmmUD/eScyA==" - }, "needle": { "version": "2.2.1", "resolved": "https://registry.npmjs.org/needle/-/needle-2.2.1.tgz", "integrity": "sha512-t/ZswCM9JTWjAdXS9VpvqhI2Ct2sL2MdY4fUXqGJaGBk13ge99ObqRksRTbBE56K+wxUXwwfZYOuZHifFW9q+Q==", "requires": { - "debug": "2.6.9", - "iconv-lite": "0.4.23", - "sax": "1.2.4" + "debug": "^2.1.2", + "iconv-lite": "^0.4.4", + "sax": "^1.2.4" } }, + "node-addon-api": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-1.6.0.tgz", + "integrity": "sha512-HEUPBHfdH4CLR1Qq4/Ek8GT/qFSvpApjJQmcYdLCL51ADU/Y11kMuFAdIevhNrPh3ylqVGA8k6vI/oi4YUAHbA==" + }, "node-pre-gyp": { "version": "0.10.0", "resolved": "https://registry.npmjs.org/node-pre-gyp/-/node-pre-gyp-0.10.0.tgz", "integrity": "sha512-G7kEonQLRbcA/mOoFoxvlMrw6Q6dPf92+t/l0DFSMuSlDoWaI9JWIyPwK0jyE1bph//CUEL65/Fz1m2vJbmjQQ==", "requires": { - "detect-libc": "1.0.3", - "mkdirp": "0.5.1", - "needle": "2.2.1", - "nopt": "4.0.1", - "npm-packlist": "1.1.10", - "npmlog": "4.1.2", - "rc": "1.2.8", - "rimraf": "2.6.2", - "semver": "5.5.0", - "tar": "4.4.4" + "detect-libc": "^1.0.2", + "mkdirp": "^0.5.1", + "needle": "^2.2.0", + "nopt": "^4.0.1", + "npm-packlist": "^1.1.6", + "npmlog": "^4.0.2", + "rc": "^1.1.7", + "rimraf": "^2.6.1", + "semver": "^5.3.0", + "tar": "^4" } }, "nopt": { @@ -720,8 +720,8 @@ "resolved": "https://registry.npmjs.org/nopt/-/nopt-4.0.1.tgz", "integrity": "sha1-0NRoWv1UFRk8jHUFYC0NF81kR00=", "requires": { - "abbrev": "1.1.1", - "osenv": "0.1.5" + "abbrev": "1", + "osenv": "^0.1.4" } }, "npm-bundled": { @@ -734,8 +734,8 @@ "resolved": "https://registry.npmjs.org/npm-packlist/-/npm-packlist-1.1.10.tgz", "integrity": "sha512-AQC0Dyhzn4EiYEfIUjCdMl0JJ61I2ER9ukf/sLxJUcZHfo+VyEfz2rMJgLZSS1v30OxPQe1cN0LZA1xbcaVfWA==", "requires": { - "ignore-walk": "3.0.1", - "npm-bundled": "1.0.3" + "ignore-walk": "^3.0.1", + "npm-bundled": "^1.0.1" } }, "npmlog": { @@ -743,10 +743,10 @@ "resolved": "https://registry.npmjs.org/npmlog/-/npmlog-4.1.2.tgz", "integrity": "sha512-2uUqazuKlTaSI/dC8AzicUck7+IrEaOnN/e0jd3Xtt1KcGpwx30v50mL7oPyr/h9bL3E4aZccVwpwP+5W9Vjkg==", "requires": { - "are-we-there-yet": "1.1.4", - "console-control-strings": "1.1.0", - "gauge": "2.7.4", - "set-blocking": "2.0.0" + "are-we-there-yet": "~1.1.2", + "console-control-strings": "~1.1.0", + "gauge": "~2.7.3", + "set-blocking": "~2.0.0" } }, "number-is-nan": { @@ -781,7 +781,7 @@ "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", "requires": { - "wrappy": "1.0.2" + "wrappy": "1" } }, "os-homedir": { @@ -799,8 +799,8 @@ "resolved": "https://registry.npmjs.org/osenv/-/osenv-0.1.5.tgz", "integrity": "sha512-0CWcCECdMVc2Rw3U5w9ZjqX6ga6ubk1xDVKxtBQPK7wis/0F2r9T6k4ydGYhecl7YUBxBVxhL5oisPsNxAPe2g==", "requires": { - "os-homedir": "1.0.2", - "os-tmpdir": "1.0.2" + "os-homedir": "^1.0.0", + "os-tmpdir": "^1.0.0" } }, "path-is-absolute": { @@ -845,10 +845,10 @@ "resolved": "https://registry.npmjs.org/rc/-/rc-1.2.8.tgz", "integrity": "sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==", "requires": { - "deep-extend": "0.6.0", - "ini": "1.3.5", - "minimist": "1.2.0", - "strip-json-comments": "2.0.1" + "deep-extend": "^0.6.0", + "ini": "~1.3.0", + "minimist": "^1.2.0", + "strip-json-comments": "~2.0.1" }, "dependencies": { "deep-extend": { @@ -863,13 +863,13 @@ "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "requires": { - "core-util-is": "1.0.2", - "inherits": "2.0.3", - "isarray": "1.0.0", - "process-nextick-args": "2.0.0", - "safe-buffer": "5.1.2", - "string_decoder": "1.1.1", - "util-deprecate": "1.0.2" + "core-util-is": "~1.0.0", + "inherits": "~2.0.3", + "isarray": "~1.0.0", + "process-nextick-args": "~2.0.0", + "safe-buffer": "~5.1.1", + "string_decoder": "~1.1.1", + "util-deprecate": "~1.0.1" }, "dependencies": { "string_decoder": { @@ -877,7 +877,7 @@ "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", "integrity": "sha512-n/ShnvDi6FHbbVfviro+WojiFzv+s8MPMHBczVePfUpDJLwoLT0ht1l4YwBCbi8pJAveEEdnkHyPyTP/mzRfwg==", "requires": { - "safe-buffer": "5.1.2" + "safe-buffer": "~5.1.0" } } } @@ -887,26 +887,26 @@ "resolved": "https://registry.npmjs.org/request/-/request-2.87.0.tgz", "integrity": "sha512-fcogkm7Az5bsS6Sl0sibkbhcKsnyon/jV1kF3ajGmF0c8HrttdKTPRT9hieOaQHA5HEq6r8OyWOo/o781C1tNw==", "requires": { - "aws-sign2": "0.7.0", - "aws4": "1.7.0", - "caseless": "0.12.0", - "combined-stream": "1.0.6", - "extend": "3.0.1", - "forever-agent": "0.6.1", - "form-data": "2.3.2", - "har-validator": "5.0.3", - "http-signature": "1.2.0", - "is-typedarray": "1.0.0", - "isstream": "0.1.2", - "json-stringify-safe": "5.0.1", - "mime-types": "2.1.18", - "oauth-sign": "0.8.2", - "performance-now": "2.1.0", - "qs": "6.5.2", - "safe-buffer": "5.1.2", - "tough-cookie": "2.3.4", - "tunnel-agent": "0.6.0", - "uuid": "3.2.1" + "aws-sign2": "~0.7.0", + "aws4": "^1.6.0", + "caseless": "~0.12.0", + "combined-stream": "~1.0.5", + "extend": "~3.0.1", + "forever-agent": "~0.6.1", + "form-data": "~2.3.1", + "har-validator": "~5.0.3", + "http-signature": "~1.2.0", + "is-typedarray": "~1.0.0", + "isstream": "~0.1.2", + "json-stringify-safe": "~5.0.1", + "mime-types": "~2.1.17", + "oauth-sign": "~0.8.2", + "performance-now": "^2.1.0", + "qs": "~6.5.1", + "safe-buffer": "^5.1.1", + "tough-cookie": "~2.3.3", + "tunnel-agent": "^0.6.0", + "uuid": "^3.1.0" } }, "resolve": { @@ -915,7 +915,7 @@ "integrity": "sha512-hgoSGrc3pjzAPHNBg+KnFcK2HwlHTs/YrAGUr6qgTVUZmXv1UEXXl0bZNBKMA9fud6lRYFdPGz0xXxycPzmmiw==", "dev": true, "requires": { - "path-parse": "1.0.5" + "path-parse": "^1.0.5" } }, "resumer": { @@ -924,7 +924,7 @@ "integrity": "sha1-8ej0YeQGS6Oegq883CqMiT0HZ1k=", "dev": true, "requires": { - "through": "2.3.8" + "through": "~2.3.4" } }, "rimraf": { @@ -932,7 +932,7 @@ "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.2.tgz", "integrity": "sha512-lreewLK/BlghmxtfH36YYVg1i8IAce4TI7oao75I1g245+6BctqTVQiBP3YUJ9C6DQOXJmkYR9X9fCLtCOJc5w==", "requires": { - "glob": "7.1.2" + "glob": "^7.0.5" } }, "safe-buffer": { @@ -970,14 +970,14 @@ "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.14.1.tgz", "integrity": "sha1-Ew9Zde3a2WPx1W+SuaxsUfqfg+s=", "requires": { - "asn1": "0.2.3", - "assert-plus": "1.0.0", - "bcrypt-pbkdf": "1.0.1", - "dashdash": "1.14.1", - "ecc-jsbn": "0.1.1", - "getpass": "0.1.7", - "jsbn": "0.1.1", - "tweetnacl": "0.14.5" + "asn1": "~0.2.3", + "assert-plus": "^1.0.0", + "bcrypt-pbkdf": "^1.0.0", + "dashdash": "^1.12.0", + "ecc-jsbn": "~0.1.1", + "getpass": "^0.1.1", + "jsbn": "~0.1.0", + "tweetnacl": "~0.14.0" } }, "string-width": { @@ -985,9 +985,9 @@ "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "requires": { - "code-point-at": "1.1.0", - "is-fullwidth-code-point": "1.0.0", - "strip-ansi": "3.0.1" + "code-point-at": "^1.0.0", + "is-fullwidth-code-point": "^1.0.0", + "strip-ansi": "^3.0.0" } }, "string.prototype.trim": { @@ -996,9 +996,9 @@ "integrity": "sha1-0E3iyJ4Tf019IG8Ia17S+ua+jOo=", "dev": true, "requires": { - "define-properties": "1.1.2", - "es-abstract": "1.11.0", - "function-bind": "1.1.1" + "define-properties": "^1.1.2", + "es-abstract": "^1.5.0", + "function-bind": "^1.0.2" } }, "strip-ansi": { @@ -1006,7 +1006,7 @@ "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "requires": { - "ansi-regex": "2.1.1" + "ansi-regex": "^2.0.0" } }, "strip-json-comments": { @@ -1020,19 +1020,19 @@ "integrity": "sha512-j0jO9BiScfqtPBb9QmPLL0qvxXMz98xjkMb7x8lKipFlJZwNJkqkWPou+NU4V6T9RnVh1kuSthLE8gLrN8bBfw==", "dev": true, "requires": { - "deep-equal": "1.0.1", - "defined": "1.0.0", - "for-each": "0.3.2", - "function-bind": "1.1.1", - "glob": "7.1.2", - "has": "1.0.1", - "inherits": "2.0.3", - "minimist": "1.2.0", - "object-inspect": "1.5.0", - "resolve": "1.5.0", - "resumer": "0.0.0", - "string.prototype.trim": "1.1.2", - "through": "2.3.8" + "deep-equal": "~1.0.1", + "defined": "~1.0.0", + "for-each": "~0.3.2", + "function-bind": "~1.1.1", + "glob": "~7.1.2", + "has": "~1.0.1", + "inherits": "~2.0.3", + "minimist": "~1.2.0", + "object-inspect": "~1.5.0", + "resolve": "~1.5.0", + "resumer": "~0.0.0", + "string.prototype.trim": "~1.1.2", + "through": "~2.3.8" } }, "tar": { @@ -1040,13 +1040,13 @@ "resolved": "https://registry.npmjs.org/tar/-/tar-4.4.4.tgz", "integrity": "sha512-mq9ixIYfNF9SK0IS/h2HKMu8Q2iaCuhDDsZhdEag/FHv8fOaYld4vN7ouMgcSSt5WKZzPs8atclTcJm36OTh4w==", "requires": { - "chownr": "1.0.1", - "fs-minipass": "1.2.5", - "minipass": "2.3.3", - "minizlib": "1.1.0", - "mkdirp": "0.5.1", - "safe-buffer": "5.1.2", - "yallist": "3.0.2" + "chownr": "^1.0.1", + "fs-minipass": "^1.2.5", + "minipass": "^2.3.3", + "minizlib": "^1.1.0", + "mkdirp": "^0.5.0", + "safe-buffer": "^5.1.2", + "yallist": "^3.0.2" }, "dependencies": { "safe-buffer": { @@ -1067,7 +1067,7 @@ "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-2.3.4.tgz", "integrity": "sha512-TZ6TTfI5NtZnuyy/Kecv+CnoROnyXn2DN97LontgQpCwsX2XyLYCC0ENhYkehSOwAp8rTQKc/NUIF7BkQ5rKLA==", "requires": { - "punycode": "1.4.1" + "punycode": "^1.4.1" } }, "tunnel-agent": { @@ -1075,7 +1075,7 @@ "resolved": "https://registry.npmjs.org/tunnel-agent/-/tunnel-agent-0.6.0.tgz", "integrity": "sha1-J6XeoGs2sEoKmWZ3SykIaPD8QP0=", "requires": { - "safe-buffer": "5.1.2" + "safe-buffer": "^5.0.1" } }, "tweetnacl": { @@ -1127,9 +1127,9 @@ "resolved": "https://registry.npmjs.org/verror/-/verror-1.10.0.tgz", "integrity": "sha1-OhBcoXBTr1XW4nDB+CiGguGNpAA=", "requires": { - "assert-plus": "1.0.0", + "assert-plus": "^1.0.0", "core-util-is": "1.0.2", - "extsprintf": "1.3.0" + "extsprintf": "^1.2.0" } }, "wide-align": { @@ -1137,7 +1137,7 @@ "resolved": "https://registry.npmjs.org/wide-align/-/wide-align-1.1.2.tgz", "integrity": "sha512-ijDLlyQ7s6x1JgCLur53osjm/UXUYD9+0PbYKrBsYisYXzCxN+HC3mYDNy/dWdmf3AwqwU3CXwDCvsNgGK1S0w==", "requires": { - "string-width": "1.0.2" + "string-width": "^1.0.2" } }, "wrappy": { @@ -1151,8 +1151,8 @@ "integrity": "sha1-F76T6q4/O3eTWceVtBlwWogX6Gg=", "dev": true, "requires": { - "sax": "1.2.4", - "xmlbuilder": "4.2.1" + "sax": ">=0.6.0", + "xmlbuilder": "^4.1.0" } }, "xmlbuilder": { @@ -1161,7 +1161,7 @@ "integrity": "sha1-qlijBBoGb5DqoWwvU4n/GfP0YaU=", "dev": true, "requires": { - "lodash": "4.17.10" + "lodash": "^4.0.0" } }, "yallist": { diff --git a/package.json b/package.json index 77469f7..362bfdb 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@mapbox/node-cpp-skel", "version": "0.1.0", - "description": "Skeleton for bindings to C++ libraries for Node.js using NAN", + "description": "Skeleton for bindings to C++ libraries for Node.js using N-API (node-addon-api)", "url": "http://github.com/mapbox/node-cpp-skel", "main": "./lib/index.js", "repository": { @@ -17,7 +17,7 @@ "license": "ISC", "dependencies": { "mason-js-sdk": "~0.1.4", - "nan": "~2.10.0", + "node-addon-api": "^1.5.0", "node-pre-gyp": "~0.10.0" }, "devDependencies": { diff --git a/src/module.cpp b/src/module.cpp index 12af606..413d8c8 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -2,26 +2,27 @@ #include "object_sync/hello.hpp" #include "standalone/hello.hpp" #include "standalone_async/hello_async.hpp" -#include +#include // #include "your_code.hpp" // "target" is a magic var that NAN_MODULE_INIT passes into a module's scope. // When you write things to target, they become available to call from // Javascript world. -NAN_MODULE_INIT(init) { +Napi::Object init(Napi::Env env, Napi::Object exports) { // expose hello method - Nan::SetMethod(target, "hello", standalone::hello); + exports.Set(Napi::String::New(env, "hello"), Napi::Function::New(env, standalone::hello)); // expose helloAsync method - Nan::SetMethod(target, "helloAsync", standalone_async::helloAsync); + exports.Set(Napi::String::New(env, "helloAsync"), Napi::Function::New(env, standalone_async::helloAsync)); // expose HelloObject class - object_sync::HelloObject::Init(target); + object_sync::HelloObject::Init(env, exports); // expose HelloObjectAsync class - object_async::HelloObjectAsync::Init(target); + object_async::HelloObjectAsync::Init(env, exports); + return exports; /** * You may have noticed there are multiple "hello" functions as part of this * module. @@ -45,4 +46,4 @@ NAN_MODULE_INIT(init) { // directly change to avoid the warning. // NODE_GYP_MODULE_NAME is the name of our module as defined in 'target_name' // variable in the 'binding.gyp', which is passed along as a compiler define -NODE_MODULE(NODE_GYP_MODULE_NAME, init) // NOLINT +NODE_API_MODULE(NODE_GYP_MODULE_NAME, init) // NOLINT diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 2a9e06b..ab85951 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -1,6 +1,6 @@ #pragma once #include -#include +#include #include namespace utils { @@ -10,7 +10,7 @@ namespace utils { * throwing errors. * Usage: * -* v8::Local callback; +* Napi::Function callback; * return CallbackError("error message", callback); // "return" is important to * prevent duplicate callbacks from being fired! * @@ -21,24 +21,25 @@ namespace utils { * context * */ -inline void CallbackError(std::string message, v8::Local func) { - Nan::Callback cb(func); - v8::Local argv[1] = {Nan::Error(message.c_str())}; - Nan::Call(cb, 1, argv); +inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) +{ + Napi::Object obj = Napi::Object::New(info.Env()); + obj.Set("message", message); + auto func = info[info.Length() - 1].As(); + return func.Call({obj}); } -inline Nan::MaybeLocal NewBufferFrom(std::unique_ptr&& ptr) { - Nan::MaybeLocal res = Nan::NewBuffer( - &(*ptr)[0], - ptr->size(), - [](char*, void* hint) { - delete static_cast(hint); - }, - ptr.get()); - if (!res.IsEmpty()) { - ptr.release(); - } - return res; +inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) +{ + std::string& str = *ptr; + Napi::Value val = Napi::Buffer::New(env, + const_cast(str.data()), + str.size(), + [](Napi::Env, char*, std::string * s) { + delete s; + }, + ptr.release()); + return val; } } // namespace utils diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 8bd7426..1f4ad4e 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -37,75 +37,13 @@ // If this was not defined within a namespace, it would be in the global scope. namespace object_async { -// Custom constructor, assigns custom name arg passed in from Javascript world. -// This constructor uses member init list via the colon, aka "direct -// initialization", which is more efficient than using assignment operators. -// This constructor is using move semantics to literally "move" the value of -// name to a new place in memory (to the "name_" variable). -// This avoids copying the value and duplicating memory allocation, which can -// negatively affect performance. -HelloObjectAsync::HelloObjectAsync(std::string&& name) - : name_(name) {} - -// Triggered from Javascript world when calling "new HelloObjectAsync(name)" -NAN_METHOD(HelloObjectAsync::New) { - if (info.IsConstructCall()) { - try { - if (info.Length() >= 1) { - if (info[0]->IsString()) { - // Don't want to risk passing a null string around, which might create unpredictable behavior. - Nan::Utf8String utf8_value(info[0]); - int len = utf8_value.length(); - if (len <= 0) { - return Nan::ThrowTypeError("arg must be a non-empty string"); - } - - /** - * This line converts a V8 string to a C++ std::string. - * In the background, it triggers memory allocation (stack allocating, but std:string is also dynamically allocating memory in the heap) - * We want to avoid heap allocation to ensure more performant code. - * See https://github.com/mapbox/cpp/blob/master/glossary.md#stack-allocation - * and https://stackoverflow.com/questions/79923/what-and-where-are-the-stack-and-heap/80113#80113 - * Also, providing the length allows the std::string constructor to avoid calculating the length internally - * and should be faster since it skips an operation. - */ - std::string name(*utf8_value, static_cast(len)); - - /** - * This line is where HelloObjectAsync takes ownership of "name" with the use of move semantics. - * Then all later usage of "name" are passed by reference (const&), but the actual home or address in memory - * will always be owned by this instance of HelloObjectAsync. Generally important to know what has ownership of an object. - * When a object/value is a member of a class (like "name"), we know the class (HelloObjectAsync) has full control of the scope of the object/value. - * This avoids the scenario of "name" being destroyed or becoming out of scope. - * - * Also, we're using "new" here to create a custom C++ class, based on node::ObjectWrap since this is a node addon. - * In this case, "new" allocates a C++ object (dynamically on the heap) and then passes ownership (control of when it gets deleted) - * to V8, the javascript engine which decides when to clean up the object based on how its’ garbage collector works. - * In other words, the memory of HelloObjectAsync is expliclty deleted via node::ObjectWrap when it's gone out of scope - * (the object needs to stay alive until the V8 garbage collector has decided it's done): - * https://github.com/nodejs/node/blob/7ec28a0a506efe9d1c03240fd028bea4a3d350da/src/node_object_wrap.h#L124 - **/ - auto self = std::make_unique(std::move(name)); // Using unique pointer to adhere to cpp core guideline: https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html - self->Wrap(info.This()); // Connects C++ object to Javascript object (this) - self.release(); // Release the ownership of self so it can be managed by wrapper - } else { - return Nan::ThrowTypeError( - "arg must be a string"); - } - } else { - return Nan::ThrowTypeError( - "must provide string arg"); - } - } catch (const std::exception& ex) { - return Nan::ThrowTypeError(ex.what()); - } - info.GetReturnValue().Set(info.This()); - } else { - return Nan::ThrowTypeError( - "Cannot call constructor as function, you need to use 'new' keyword"); - } -} +// This is the worker running asynchronously and calling a user-provided +// callback when done. +// Consider storing all C++ objects you need by value or by shared_ptr to keep +// them alive until done. +// Nan AsyncWorker docs: +// https://github.com/nodejs/nan/blob/master/doc/asyncworker.md // This function performs expensive allocation of std::map, querying, and string // comparison, therefore threads are nice & busy. @@ -142,26 +80,17 @@ std::unique_ptr do_expensive_work(bool louder, std::string const& n return result; } -// This is the worker running asynchronously and calling a user-provided -// callback when done. -// Consider storing all C++ objects you need by value or by shared_ptr to keep -// them alive until done. -// Nan AsyncWorker docs: -// https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Nan::AsyncWorker // NOLINT to disable cppcoreguidelines-special-member-functions +struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelines-special-member-functions { - using Base = Nan::AsyncWorker; - // We explicitly delete the copy constructor and assignment operator below (even though Nan::Asyncworker) - // already does this in the base class. This allows us to have the `const std::string* name` - // pointer member without the silly g++ warning of "error: ‘struct object_async::AsyncHelloWorker’ has pointer data members [-Werror=effc++]" + using Base = Napi::AsyncWorker; AsyncHelloWorker(AsyncHelloWorker const&) = delete; AsyncHelloWorker& operator=(AsyncHelloWorker const&) = delete; AsyncHelloWorker(bool louder, bool buffer, - const std::string* name, - Nan::Callback* cb) - : Base(cb, "skel:object-async-worker"), + std::string const* name, + Napi::Function& cb) + : Base(cb), louder_(louder), buffer_(buffer), name_(name) {} @@ -170,162 +99,137 @@ struct AsyncHelloWorker : Nan::AsyncWorker // NOLINT to disable cppcoreguideline // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. void Execute() override { - try { + try + { result_ = do_expensive_work(louder_, *name_); - } catch (const std::exception& e) { - SetErrorMessage(e.what()); + } + catch (std::exception const& e) + { + SetError(e.what()); } } - // The HandleOKCallback() is getting called when Execute() successfully + // The OnOK() is getting called when Execute() successfully // completed. // - In case Execute() invoked SetErrorMessage("") this function is not // getting called. // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void HandleOKCallback() override { - Nan::HandleScope scope; - - if (buffer_) { - const auto argc = 2u; - v8::Local argv[argc] = { - Nan::Null(), utils::NewBufferFrom(std::move(result_)).ToLocalChecked()}; - - // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy - callback->Call(argc, static_cast*>(argv), async_resource); - - } else { - const auto argc = 2u; - v8::Local argv[argc] = { - Nan::Null(), Nan::New(*result_).ToLocalChecked()}; - - // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy - callback->Call(argc, static_cast*>(argv), async_resource); + void OnOK() override + { + Napi::HandleScope scope(Env()); + if (buffer_) + { + Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); + } + else + { + Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } std::unique_ptr result_ = std::make_unique(); - const bool louder_; - const bool buffer_; + bool const louder_; + bool const buffer_; // We use a pointer here to avoid copying the string data. // This works because we know that the original string we are // pointing to will be kept in scope/alive for the time while AsyncHelloWorker // is using it. If we could not guarantee this then we would need to either // copy the string or pass a shared_ptr. - const std::string* name_; + std::string const* name_; }; -NAN_METHOD(HelloObjectAsync::helloAsync) { - // "info" comes from the NAN_METHOD macro, which returns differently according - // to the Node version - // "What is node::ObjectWrap???" The short version is that node::ObjectWrap - // and wrapping/unwrapping objects - // is the somewhat clumsy way it is possible to bind Node and C++. The main - // points to remember: - // - To access a class instance inside a C++ static method, you must unwrap - // the object. - // - The C++ methods must be static to make them available at startup across - // the language boundary (JS <-> C++). - auto* h = - Nan::ObjectWrap::Unwrap(info.Holder()); +Napi::FunctionReference HelloObjectAsync::constructor; +HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) + : Napi::ObjectWrap(info) +{ + Napi::Env env = info.Env(); + Napi::HandleScope scope(env); + + std::size_t length = info.Length(); + if (length != 1 || !info[0].IsString()) + { + Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); + } + name_ = info[0].As().Utf8Value(); + if (name_.empty()) + { + Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); + } +} + +// NAN_METHOD is applicable to methods you want to expose to JS world +Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) +{ + // ???? bool louder = false; bool buffer = false; + // ???? - // Check second argument, should be a 'callback' function. - // This allows us to set the callback so we can use it to return errors - // instead of throwing. - // Also, "info" comes from the NAN_METHOD macro, which returns differently - // according to the version of node - if (!info[1]->IsFunction()) { - return Nan::ThrowTypeError("second arg 'callback' must be a function"); + Napi::Env env = info.Env(); + if (info.Length() != 2 || !info[1].IsFunction()) + { + Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); + return env.Null(); } - v8::Local callback = info[1].As(); + + Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0]->IsObject()) { - return utils::CallbackError("first arg 'options' must be an object", - callback); + if (!info[0].IsObject()) + { + return utils::CallbackError("first arg 'options' must be an object", info); } - v8::Local options = info[0].As(); + Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options->Has(Nan::New("louder").ToLocalChecked())) { - v8::Local louder_val = - options->Get(Nan::New("louder").ToLocalChecked()); - if (!louder_val->IsBoolean()) { - return utils::CallbackError("option 'louder' must be a boolean", - callback); + if (options.Has(Napi::String::New(env, "louder"))) + { + Napi::Value louder_val = options.Get(Napi::String::New(env, "louder")); + if (!louder_val.IsBoolean()) + { + return utils::CallbackError("option 'louder' must be a boolean", info); } - louder = louder_val->BooleanValue(); + louder = louder_val.As().Value(); } // Check options object for the "buffer" property, which should be a boolean // value - if (options->Has(Nan::New("buffer").ToLocalChecked())) { - v8::Local buffer_val = - options->Get(Nan::New("buffer").ToLocalChecked()); - if (!buffer_val->IsBoolean()) { - return utils::CallbackError("option 'buffer' must be a boolean", - callback); + if (options.Has(Napi::String::New(env, "buffer"))) + { + Napi::Value buffer_val = options.Get(Napi::String::New(env, "buffer")); + if (!buffer_val.IsBoolean()) + { + return utils::CallbackError("option 'buffer' must be a boolean", info); } - buffer = buffer_val->BooleanValue(); + buffer = buffer_val.As().Value(); } - // Create a worker instance and queues it to run asynchronously invoking the - // callback when done. - // - Nan::AsyncWorker takes a pointer to a Nan::Callback and deletes the - // pointer automatically. - // - Nan::AsyncQueueWorker takes a pointer to a Nan::AsyncWorker and deletes - // the pointer automatically. - auto cb = std::make_unique(callback); - auto worker = std::make_unique(louder, buffer, &h->name_, cb.release()); - Nan::AsyncQueueWorker(worker.release()); + auto * worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; + worker->Queue(); + return info.Env().Undefined(); } -// Singleton -Nan::Persistent& HelloObjectAsync::create_once() { - static Nan::Persistent init; - return init; -} +Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) +{ -void HelloObjectAsync::Init(v8::Local target) { - // A handlescope is needed so that v8 objects created in the local memory - // space (this function in this case) - // are cleaned up when the function is done running (and the handlescope is - // destroyed) - // Fun trivia: forgetting a handlescope is one of the most common causes of - // memory leaks in node.js core - // https://www.joyent.com/blog/walmart-node-js-memory-leak - Nan::HandleScope scope; - - // This is saying: - // "Node, please allocate a new Javascript string object - // inside the V8 local memory space, with the value 'HelloObjectAsync' " - v8::Local whoami = Nan::New("HelloObjectAsync").ToLocalChecked(); - - // Create the HelloObject - auto fnTp = Nan::New( - HelloObjectAsync::New, v8::Local()); // Passing the HelloObject::New method above - fnTp->InstanceTemplate()->SetInternalFieldCount(1); // It's 1 when holding the ObjectWrap itself and nothing else - fnTp->SetClassName(whoami); // Passing the Javascript string object above - - // Add custom methods here. - // This is how helloAsync() is exposed as part of HelloObjectAsync. - // This line is attaching the "helloAsync" method to a JavaScript function - // prototype. - // "helloAsync" is therefore like a property of the fnTp object - // ex: console.log(HelloObjectAsync.helloAsync) --> [Function: helloAsync] - SetPrototypeMethod(fnTp, "helloAsync", helloAsync); - - // Create an unique instance of the HelloObject function template, - // then set this unique instance to the target - const auto fn = Nan::GetFunction(fnTp).ToLocalChecked(); - create_once().Reset(fn); // calls the static &HelloObjectAsync::create_once - // method above. This ensures the instructions in - // this Init function are retained in memory even - // after this code block ends. - Nan::Set(target, whoami, fn); + Napi::Function func = DefineClass(env, "HelloObjectAsync", { + InstanceMethod("helloAsync", &HelloObjectAsync::helloAsync) + }); + // Create a peristent reference to the class constructor. This will allow + // a function called on a class prototype and a function + // called on instance of a class to be distinguished from each other. + constructor = Napi::Persistent(func); + // Call the SuppressDestruct() method on the static data prevent the calling + // to this destructor to reset the reference when the environment is no longer + // available. + constructor.SuppressDestruct(); + exports.Set("HelloObjectAsync", func); + return exports; } + + + } // namespace object_async diff --git a/src/object_async/hello_async.hpp b/src/object_async/hello_async.hpp index 3daffa1..4693d2e 100644 --- a/src/object_async/hello_async.hpp +++ b/src/object_async/hello_async.hpp @@ -1,5 +1,5 @@ #pragma once -#include +#include namespace object_async { @@ -10,27 +10,17 @@ namespace object_async { * Also, this class adheres to the rule of Zero because we define no custom * destructor or copy constructor */ -class HelloObjectAsync : public Nan::ObjectWrap { - +class HelloObjectAsync : public Napi::ObjectWrap +{ public: // initializer - static void Init(v8::Local target); - - // methods required for the V8 constructor - static NAN_METHOD(New); - static Nan::Persistent& create_once(); - - // helloAsync, custom async method tied to Init of this class - // method's logic lives in ./hello.cpp - static NAN_METHOD(helloAsync); - - // C++ Constructor - // Passing the arg by rvalue reference (&&) - HelloObjectAsync(std::string&& name); - + static Napi::Object Init(Napi::Env env, Napi::Object exports); + HelloObjectAsync(Napi::CallbackInfo const& info); + Napi::Value helloAsync(Napi::CallbackInfo const& info); private: // member variable // specific to each instance of the class + static Napi::FunctionReference constructor; std::string name_; }; } // namespace object_async diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index bbfec31..cdcde38 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -1,5 +1,4 @@ #include "hello.hpp" - #include /** @@ -28,143 +27,49 @@ // clearly organize your application. namespace object_sync { -// Custom constructor, assigns custom name passed in from Javascript world. -// This constructor uses member init list via the semicolon, aka "direct initialization" -// which is more efficient than using assignment operators. -HelloObject::HelloObject(std::string&& name) : name_(name) {} - -// Triggered from Javascript world when calling "new HelloObject(name)" -NAN_METHOD(HelloObject::New) { - if (info.IsConstructCall()) { - try { - if (info.Length() >= 1) { - if (info[0]->IsString()) { - // Don't want to risk passing a null string around, which might create unpredictable behavior. - Nan::Utf8String utf8_value(info[0]); - int len = utf8_value.length(); - if (len <= 0) { - return Nan::ThrowTypeError("arg must be a non-empty string"); - } - - /** - * This line converts a V8 string to a C++ std::string. - * In the background, it triggers memory allocation (stack allocating, but std:string is also dynamically allocating memory in the heap) - * We want to avoid heap allocation to ensure more performant code. - * See https://github.com/mapbox/cpp/blob/master/glossary.md#stack-allocation - * and https://stackoverflow.com/questions/79923/what-and-where-are-the-stack-and-heap/80113#80113 - * Also, providing the length allows the std::string constructor to avoid calculating the length internally - * and should be faster since it skips an operation. - */ - std::string name(*utf8_value, static_cast(len)); - /** - * This line is where HelloObject takes ownership of "name" with the use of move semantics. - * Then all later usage of "name" are passed by reference (const&), but the actual home or address in memory - * will always be owned by this instance of HelloObjectAsync. Generally important to know what has ownership of an object. - * When a object/value is a member of a class (like "name"), we know the class (HelloObjectAsync) has full control of the scope of the object/value. - * This avoids the scenario of "name" being destroyed or becoming out of scope. - * - * Also, we're using "new" here to create a custom C++ class, based on node::ObjectWrap since this is a node addon. - * In this case, "new" allocates a C++ object (dynamically on the heap) and then passes ownership (control of when it gets deleted) - * to V8, the javascript engine which decides when to clean up the object based on how its’ garbage collector works. - * In other words, the memory of HelloObjectAsync is expliclty deleted via node::ObjectWrap when it's gone out of scope - * (the object needs to stay alive until the V8 garbage collector has decided it's done): - * https://github.com/nodejs/node/blob/7ec28a0a506efe9d1c03240fd028bea4a3d350da/src/node_object_wrap.h#L124 - */ - auto self = std::make_unique(std::move(name)); // Using unique pointer to adhere to cpp core guideline: https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html - self->Wrap(info.This()); // Connects C++ object to Javascript object (this) - self.release(); // Release the ownership of self so it can be managed by wrapper - } else { - return Nan::ThrowTypeError( - "arg must be a string"); - } - } else { - return Nan::ThrowTypeError( - "must provide string arg"); - } - } catch (const std::exception& ex) { - return Nan::ThrowTypeError(ex.what()); - } +Napi::FunctionReference HelloObject::constructor; - info.GetReturnValue().Set(info.This()); - } else { - return Nan::ThrowTypeError( - "Cannot call constructor as function, you need to use 'new' keyword"); +// Triggered from Javascript world when calling "new HelloObject(name)" +HelloObject::HelloObject(Napi::CallbackInfo const& info) + : Napi::ObjectWrap(info) +{ + Napi::Env env = info.Env(); + Napi::HandleScope scope(env); + std::size_t length = info.Length(); + if (length != 1 || !info[0].IsString()) + { + Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); + } + name_ = info[0].As().Utf8Value(); + if (name_.empty()) + { + Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); } } -// NAN_METHOD is applicable to methods you want to expose to JS world -NAN_METHOD(HelloObject::hello) { - /** - * Note: a HandleScope is automatically included inside NAN_METHOD. See the - * docs at NAN that say: - * 'Note that an implicit HandleScope is created for you on - * JavaScript-accessible methods so you do not need to insert one yourself.' - * at - * https://github.com/nodejs/nan/blob/2dfc5c2d19c8066903a19ced6a72c06d2c825dec/doc/scopes.md#nanhandlescope - - * "What is node::ObjectWrap???" The short version is that node::ObjectWrap - * and wrapping/unwrapping objects - * is the somewhat clumsy way it is possible to bind Node and C++. The main - * points to remember: - * - To access a class instance inside a C++ static method, you must unwrap - * the object. - * - The C++ methods must be static to make them available at startup across - * the language boundary (JS <-> C++). - */ - auto* h = Nan::ObjectWrap::Unwrap(info.Holder()); - - // "info" comes from the NAN_METHOD macro, which returns differently - // according to the version of node - info.GetReturnValue().Set( - Nan::New("...initialized an object...hello " + h->name_) - .ToLocalChecked()); -} - -// This is a Singleton, which is a general programming design concept for -// creating an instance once within a process. -Nan::Persistent& HelloObject::create_once() { - static Nan::Persistent init; - return init; +Napi::Value HelloObject::hello(Napi::CallbackInfo const& info) +{ + Napi::Env env = info.Env(); + return Napi::String::New(env, name_); } -void HelloObject::Init(v8::Local target) { - // A handlescope is needed so that v8 objects created in the local memory - // space (this function in this case) - // are cleaned up when the function is done running (and the handlescope is - // destroyed) - // Fun trivia: forgetting a handlescope is one of the most common causes of - // memory leaks in node.js core - // https://www.joyent.com/blog/walmart-node-js-memory-leak - Nan::HandleScope scope; - - // This is saying: - // "Node, please allocate a new Javascript string object - // inside the V8 local memory space, with the value 'HelloObject' " - v8::Local whoami = Nan::New("HelloObject").ToLocalChecked(); - - // Officially create the HelloObject - auto fnTp = Nan::New( - HelloObject::New, v8::Local()); // Passing the HelloObject::New method above - fnTp->InstanceTemplate()->SetInternalFieldCount(1); // It's 1 when holding the ObjectWrap itself and nothing else - fnTp->SetClassName(whoami); // Passing the Javascript string object above - - // Add custom methods here. - // This is how hello() is exposed as part of HelloObject. - // This line is attaching the "hello" method to a JavaScript function - // prototype. - // "hello" is therefore like a property of the fnTp object - // ex: console.log(HelloObject.hello) --> [Function: hello] - SetPrototypeMethod(fnTp, "helloMethod", hello); - - // Create an unique instance of the HelloObject function template, - // then set this unique instance to the target - const auto fn = Nan::GetFunction(fnTp).ToLocalChecked(); - create_once().Reset(fn); // calls the static &HelloObject::create_once method - // above. This ensures the instructions in this Init - // function are retained in memory even after this - // code block ends. - Nan::Set(target, whoami, fn); +Napi::Object HelloObject::Init(Napi::Env env, Napi::Object exports) +{ + + Napi::Function func = DefineClass(env, "HelloObject", { + InstanceMethod("helloMethod", &HelloObject::hello) + }); + // Create a peristent reference to the class constructor. This will allow + // a function called on a class prototype and a function + // called on instance of a class to be distinguished from each other. + constructor = Napi::Persistent(func); + // Call the SuppressDestruct() method on the static data prevent the calling + // to this destructor to reset the reference when the environment is no longer + // available. + constructor.SuppressDestruct(); + exports.Set("HelloObject", func); + return exports; } } // namespace object_sync diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index a556cfc..13c7df8 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include namespace object_sync { @@ -9,27 +9,15 @@ namespace object_sync { * This is in a header file so we can access it across other .cpp files if necessary * Also, this class adheres to the rule of Zero because we define no custom destructor or copy constructor */ -class HelloObject : public Nan::ObjectWrap { - +class HelloObject : public Napi::ObjectWrap +{ public: - // initializer - static void Init(v8::Local target); - - // methods required for the V8 constructor (?) - static NAN_METHOD(New); - static Nan::Persistent& create_once(); - - // hello, custom sync method tied to Init of this class - // method's logic lives in ./hello.cpp - static NAN_METHOD(hello); - - // C++ Constructor - // Passing the arg by rvalue reference (&&) - HelloObject(std::string&& name); - + // initializers + static Napi::Object Init(Napi::Env env, Napi::Object exports); + HelloObject(Napi::CallbackInfo const& info); + Napi::Value hello(Napi::CallbackInfo const& info); private: - // member variable - // specific to each instance of the class + static Napi::FunctionReference constructor; std::string name_; }; } // namespace object_sync diff --git a/src/standalone/hello.cpp b/src/standalone/hello.cpp index 2a21c0f..7024295 100644 --- a/src/standalone/hello.cpp +++ b/src/standalone/hello.cpp @@ -19,11 +19,11 @@ namespace standalone { // hello is a "standalone function" because it's not a class. // If this function was not defined within a namespace, it would be in the // global scope. -NAN_METHOD(hello) { +Napi::Value hello(Napi::CallbackInfo const& info) { // "info" comes from the NAN_METHOD macro, which returns differently // according to the version of node - info.GetReturnValue().Set( - Nan::New("hello world").ToLocalChecked()); + Napi::Env env = info.Env(); + return Napi::String::New(env, "hello world"); } -} // namespace standalone \ No newline at end of file +} // namespace standalone diff --git a/src/standalone/hello.hpp b/src/standalone/hello.hpp index d445cae..d461cf1 100644 --- a/src/standalone/hello.hpp +++ b/src/standalone/hello.hpp @@ -1,6 +1,7 @@ #pragma once -#include -// specify #include with carrots, ex: --> look for header in global +#include + +// specify #include with carrots, ex: --> look for header in global // specify #include with quotes, ex: "hello.hpp" --> look for header in location // relative to this file @@ -32,5 +33,5 @@ namespace standalone { // hello, custom sync method tied to module.cpp // method's logic lives in hello.cpp -NAN_METHOD(hello); -} // namespace standalone \ No newline at end of file +Napi::Value hello(Napi::CallbackInfo const& info); +} // namespace standalone diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 0dfba5e..0422f92 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -70,11 +70,14 @@ std::unique_ptr do_expensive_work(bool louder) { // them alive until done. // Nan AsyncWorker docs: // https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Nan::AsyncWorker { - using Base = Nan::AsyncWorker; +struct AsyncHelloWorker : Napi::AsyncWorker +{ + using Base = Napi::AsyncWorker; - AsyncHelloWorker(bool louder, bool buffer, Nan::Callback* cb) - : Base(cb, "skel:standalone-async-worker"), louder_(louder), buffer_(buffer) {} + AsyncHelloWorker(bool louder, bool buffer, Napi::Function& cb) + : Base(cb), + louder_(louder), + buffer_(buffer) {} // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. @@ -82,35 +85,33 @@ struct AsyncHelloWorker : Nan::AsyncWorker { void Execute() override { // The try/catch is critical here: if code was added that could throw an // unhandled error INSIDE the threadpool, it would be disasterous - try { + try + { result_ = do_expensive_work(louder_); - } catch (const std::exception& e) { - SetErrorMessage(e.what()); + } + catch (std::exception const& e) + { + SetError(e.what()); } } - // The HandleOKCallback() is getting called when Execute() successfully + // The OnOK() is getting called when Execute() successfully // completed. // - In case Execute() invoked SetErrorMessage("") this function is not // getting called. // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void HandleOKCallback() override { - Nan::HandleScope scope; - - if (buffer_) { - const auto argc = 2u; - v8::Local argv[argc] = { - Nan::Null(), utils::NewBufferFrom(std::move(result_)).ToLocalChecked()}; - // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy - callback->Call(argc, static_cast*>(argv), async_resource); - } else { - const auto argc = 2u; - v8::Local argv[argc] = { - Nan::Null(), Nan::New(*result_).ToLocalChecked()}; - // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy - callback->Call(argc, static_cast*>(argv), async_resource); + void OnOK() override + { + Napi::HandleScope scope(Env()); + if (buffer_) + { + Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); + } + else + { + Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } @@ -122,8 +123,8 @@ struct AsyncHelloWorker : Nan::AsyncWorker { // helloAsync is a "standalone function" because it's not a class. // If this function was not defined within a namespace ("standalone_async" // specified above), it would be in the global scope. -NAN_METHOD(helloAsync) { - +Napi::Value helloAsync(Napi::CallbackInfo const& info) +{ bool louder = false; bool buffer = false; @@ -132,50 +133,53 @@ NAN_METHOD(helloAsync) { // instead of throwing. // Also, "info" comes from the NAN_METHOD macro, which returns differently // according to the version of node - if (!info[1]->IsFunction()) { - return Nan::ThrowTypeError("second arg 'callback' must be a function"); + if (!info[1].IsFunction()) + { + Napi::TypeError::New(info.Env(), "second arg 'callback' must be a function").ThrowAsJavaScriptException(); + return info.Env().Null(); } - v8::Local callback = info[1].As(); + + Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0]->IsObject()) { - return utils::CallbackError("first arg 'options' must be an object", - callback); + if (!info[0].IsObject()) + { + return utils::CallbackError("first arg 'options' must be an object", info); } - v8::Local options = info[0].As(); + Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options->Has(Nan::New("louder").ToLocalChecked())) { - v8::Local louder_val = - options->Get(Nan::New("louder").ToLocalChecked()); - if (!louder_val->IsBoolean()) { - return utils::CallbackError("option 'louder' must be a boolean", - callback); + if (options.Has(Napi::String::New(info.Env(), "louder"))) + { + Napi::Value louder_val = options.Get(Napi::String::New(info.Env(), "louder")); + if (!louder_val.IsBoolean()) + { + return utils::CallbackError("option 'louder' must be a boolean", info); } - louder = louder_val->BooleanValue(); + louder = louder_val.As().Value(); } // Check options object for the "buffer" property, which should be a boolean // value - if (options->Has(Nan::New("buffer").ToLocalChecked())) { - v8::Local buffer_val = - options->Get(Nan::New("buffer").ToLocalChecked()); - if (!buffer_val->IsBoolean()) { - return utils::CallbackError("option 'buffer' must be a boolean", - callback); + if (options.Has(Napi::String::New(info.Env(), "buffer"))) + { + Napi::Value buffer_val = options.Get(Napi::String::New(info.Env(), "buffer")); + if (!buffer_val.IsBoolean()) + { + return utils::CallbackError("option 'buffer' must be a boolean", info); } - buffer = buffer_val->BooleanValue(); + buffer = buffer_val.As().Value(); } // Creates a worker instance and queues it to run asynchronously, invoking the // callback when done. - // - Nan::AsyncWorker takes a pointer to a Nan::Callback and deletes the + // - Napi::AsyncWorker takes a pointer to a Napi::FunctionReference and deletes the // pointer automatically. - // - Nan::AsyncQueueWorker takes a pointer to a Nan::AsyncWorker and deletes + // - Napi::AsyncQueueWorker takes a pointer to a Napi::AsyncWorker and deletes // the pointer automatically. - auto cb = std::make_unique(callback); - auto worker = std::make_unique(louder, buffer, cb.release()); - Nan::AsyncQueueWorker(worker.release()); + auto * worker = new AsyncHelloWorker{louder, buffer, callback}; + worker->Queue(); + return info.Env().Undefined(); } } // namespace standalone_async diff --git a/src/standalone_async/hello_async.hpp b/src/standalone_async/hello_async.hpp index e846c42..6e1a786 100644 --- a/src/standalone_async/hello_async.hpp +++ b/src/standalone_async/hello_async.hpp @@ -1,6 +1,6 @@ #pragma once -#include -// carrots, ex: --> look for header in global +#include +// carrots, ex: --> look for header in global // quotes, ex: "hello.hpp" --> look for header in location relative to this file /* @@ -31,5 +31,5 @@ namespace standalone_async { // hello, custom sync method tied to module.cpp // method's logic lives in hello.cpp -NAN_METHOD(helloAsync); -} // namespace standalone_async \ No newline at end of file +Napi::Value helloAsync(Napi::CallbackInfo const& info); +} // namespace standalone_async diff --git a/test/hello_object.test.js b/test/hello_object.test.js index 07a943c..3589064 100644 --- a/test/hello_object.test.js +++ b/test/hello_object.test.js @@ -4,7 +4,7 @@ var module = require('../lib/index.js'); test('success: prints expected string', function(t) { var H = new module.HelloObject('carol'); var check = H.helloMethod(); - t.equal(check, '...initialized an object...hello carol', 'returned expected string'); + t.equal(check, 'carol', 'returned expected string'); t.end(); }); @@ -13,7 +13,7 @@ test('error: throws when passing empty string', function(t) { var H = new module.HelloObject(''); } catch(err) { t.ok(err, 'expected error'); - t.equal(err.message, 'arg must be a non-empty string', 'expected error message') + t.equal(err.message, 'arg must be a non-empty string', 'expected error message'); t.end(); } }); @@ -23,7 +23,7 @@ test('error: throws when missing "new"', function(t) { var H = module.HelloObject(); } catch(err) { t.ok(err); - t.equal(err.message, 'Cannot call constructor as function, you need to use \'new\' keyword', 'expected error message') + t.equal(err.message, 'Class constructors cannot be invoked without \'new\'', 'expected error message'); t.end(); } }); @@ -33,7 +33,7 @@ test('error: handles non-string arg within constructor', function(t) { var H = new module.HelloObject(24); } catch(err) { t.ok(err, 'expected error'); - t.ok(err.message.indexOf('arg must be a string') > -1, 'expected error message'); + t.ok(err.message, 'A string was expected', 'expected error message'); t.end(); } }); @@ -43,7 +43,7 @@ test('error: handles missing arg', function(t) { var H = new module.HelloObject(); } catch (err) { t.ok(err, 'expected error'); - t.ok(err.message.indexOf('must provide string arg') > -1, 'expected error message'); + t.ok(err.message, 'must provide string arg', 'expected error message'); t.end(); } -}); \ No newline at end of file +}); diff --git a/test/hello_object_async.test.js b/test/hello_object_async.test.js index 74452c7..ec2400e 100644 --- a/test/hello_object_async.test.js +++ b/test/hello_object_async.test.js @@ -45,7 +45,7 @@ test('error: throws when missing "new"', function(t) { var H = module.HelloObjectAsync('world'); } catch(err) { t.ok(err, 'expected error'); - t.equal(err.message, 'Cannot call constructor as function, you need to use \'new\' keyword', 'expected error message') + t.equal(err.message, 'Class constructors cannot be invoked without \'new\'', 'expected error message') t.end(); } }); @@ -54,8 +54,8 @@ test('error: handles non-string arg within constructor', function(t) { try { var H = new module.HelloObjectAsync(24); } catch(err) { - t.ok(err, 'expected error'); - t.ok(err.message.indexOf('arg must be a string') > -1, 'expected error message'); + console.log(err.message); + t.ok(err.message.indexOf('A string was expected') > -1, 'expected error message'); t.end(); } }); @@ -103,7 +103,7 @@ test('error: handles missing arg', function(t) { var H = new module.HelloObjectAsync(); } catch (err) { t.ok(err, 'expected error'); - t.ok(err.message.indexOf('must provide string arg') > -1, 'expected error message'); + t.ok(err.message.indexOf('A string was expected') > -1, 'expected error message'); t.end(); } }); From 8e796cea722b26b03bf45868ec6e8a7ed45fd715 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 17:23:28 +0000 Subject: [PATCH 02/55] clang-format --- src/module_utils.hpp | 10 ++--- src/object_async/hello_async.cpp | 59 +++++++++------------------- src/object_async/hello_async.hpp | 4 +- src/object_sync/hello.cpp | 20 +++------- src/object_sync/hello.hpp | 4 +- src/standalone_async/hello_async.cpp | 43 +++++++------------- 6 files changed, 46 insertions(+), 94 deletions(-) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index ab85951..925a768 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -21,22 +21,20 @@ namespace utils { * context * */ -inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) -{ +inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) { Napi::Object obj = Napi::Object::New(info.Env()); obj.Set("message", message); auto func = info[info.Length() - 1].As(); return func.Call({obj}); } -inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) -{ +inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) { std::string& str = *ptr; Napi::Value val = Napi::Buffer::New(env, const_cast(str.data()), str.size(), - [](Napi::Env, char*, std::string * s) { - delete s; + [](Napi::Env, char*, std::string* s) { + delete s; }, ptr.release()); return val; diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 1f4ad4e..9caaa79 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -37,7 +37,6 @@ // If this was not defined within a namespace, it would be in the global scope. namespace object_async { - // This is the worker running asynchronously and calling a user-provided // callback when done. // Consider storing all C++ objects you need by value or by shared_ptr to keep @@ -99,12 +98,9 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. void Execute() override { - try - { + try { result_ = do_expensive_work(louder_, *name_); - } - catch (std::exception const& e) - { + } catch (std::exception const& e) { SetError(e.what()); } } @@ -116,15 +112,11 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override - { + void OnOK() override { Napi::HandleScope scope(Env()); - if (buffer_) - { + if (buffer_) { Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); - } - else - { + } else { Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } @@ -143,34 +135,29 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin Napi::FunctionReference HelloObjectAsync::constructor; HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) -{ + : Napi::ObjectWrap(info) { Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); - if (length != 1 || !info[0].IsString()) - { + if (length != 1 || !info[0].IsString()) { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); } name_ = info[0].As().Utf8Value(); - if (name_.empty()) - { + if (name_.empty()) { Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); } } // NAN_METHOD is applicable to methods you want to expose to JS world -Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) -{ +Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { // ???? bool louder = false; bool buffer = false; // ???? Napi::Env env = info.Env(); - if (info.Length() != 2 || !info[1].IsFunction()) - { + if (info.Length() != 2 || !info[1].IsFunction()) { Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return env.Null(); } @@ -178,46 +165,38 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0].IsObject()) - { + if (!info[0].IsObject()) { return utils::CallbackError("first arg 'options' must be an object", info); } Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options.Has(Napi::String::New(env, "louder"))) - { + if (options.Has(Napi::String::New(env, "louder"))) { Napi::Value louder_val = options.Get(Napi::String::New(env, "louder")); - if (!louder_val.IsBoolean()) - { + if (!louder_val.IsBoolean()) { return utils::CallbackError("option 'louder' must be a boolean", info); } louder = louder_val.As().Value(); } // Check options object for the "buffer" property, which should be a boolean // value - if (options.Has(Napi::String::New(env, "buffer"))) - { + if (options.Has(Napi::String::New(env, "buffer"))) { Napi::Value buffer_val = options.Get(Napi::String::New(env, "buffer")); - if (!buffer_val.IsBoolean()) - { + if (!buffer_val.IsBoolean()) { return utils::CallbackError("option 'buffer' must be a boolean", info); } buffer = buffer_val.As().Value(); } - auto * worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; + auto* worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; worker->Queue(); return info.Env().Undefined(); } -Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) -{ +Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) { - Napi::Function func = DefineClass(env, "HelloObjectAsync", { - InstanceMethod("helloAsync", &HelloObjectAsync::helloAsync) - }); + Napi::Function func = DefineClass(env, "HelloObjectAsync", {InstanceMethod("helloAsync", &HelloObjectAsync::helloAsync)}); // Create a peristent reference to the class constructor. This will allow // a function called on a class prototype and a function // called on instance of a class to be distinguished from each other. @@ -230,6 +209,4 @@ Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) return exports; } - - } // namespace object_async diff --git a/src/object_async/hello_async.hpp b/src/object_async/hello_async.hpp index 4693d2e..fcbcd0f 100644 --- a/src/object_async/hello_async.hpp +++ b/src/object_async/hello_async.hpp @@ -10,13 +10,13 @@ namespace object_async { * Also, this class adheres to the rule of Zero because we define no custom * destructor or copy constructor */ -class HelloObjectAsync : public Napi::ObjectWrap -{ +class HelloObjectAsync : public Napi::ObjectWrap { public: // initializer static Napi::Object Init(Napi::Env env, Napi::Object exports); HelloObjectAsync(Napi::CallbackInfo const& info); Napi::Value helloAsync(Napi::CallbackInfo const& info); + private: // member variable // specific to each instance of the class diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index cdcde38..7b52814 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -27,39 +27,31 @@ // clearly organize your application. namespace object_sync { - Napi::FunctionReference HelloObject::constructor; // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) -{ + : Napi::ObjectWrap(info) { Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); - if (length != 1 || !info[0].IsString()) - { + if (length != 1 || !info[0].IsString()) { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); } name_ = info[0].As().Utf8Value(); - if (name_.empty()) - { + if (name_.empty()) { Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); } } -Napi::Value HelloObject::hello(Napi::CallbackInfo const& info) -{ +Napi::Value HelloObject::hello(Napi::CallbackInfo const& info) { Napi::Env env = info.Env(); return Napi::String::New(env, name_); } -Napi::Object HelloObject::Init(Napi::Env env, Napi::Object exports) -{ +Napi::Object HelloObject::Init(Napi::Env env, Napi::Object exports) { - Napi::Function func = DefineClass(env, "HelloObject", { - InstanceMethod("helloMethod", &HelloObject::hello) - }); + Napi::Function func = DefineClass(env, "HelloObject", {InstanceMethod("helloMethod", &HelloObject::hello)}); // Create a peristent reference to the class constructor. This will allow // a function called on a class prototype and a function // called on instance of a class to be distinguished from each other. diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index 13c7df8..9793fb7 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -9,13 +9,13 @@ namespace object_sync { * This is in a header file so we can access it across other .cpp files if necessary * Also, this class adheres to the rule of Zero because we define no custom destructor or copy constructor */ -class HelloObject : public Napi::ObjectWrap -{ +class HelloObject : public Napi::ObjectWrap { public: // initializers static Napi::Object Init(Napi::Env env, Napi::Object exports); HelloObject(Napi::CallbackInfo const& info); Napi::Value hello(Napi::CallbackInfo const& info); + private: static Napi::FunctionReference constructor; std::string name_; diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 0422f92..038d394 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -70,8 +70,7 @@ std::unique_ptr do_expensive_work(bool louder) { // them alive until done. // Nan AsyncWorker docs: // https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Napi::AsyncWorker -{ +struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; AsyncHelloWorker(bool louder, bool buffer, Napi::Function& cb) @@ -85,12 +84,9 @@ struct AsyncHelloWorker : Napi::AsyncWorker void Execute() override { // The try/catch is critical here: if code was added that could throw an // unhandled error INSIDE the threadpool, it would be disasterous - try - { + try { result_ = do_expensive_work(louder_); - } - catch (std::exception const& e) - { + } catch (std::exception const& e) { SetError(e.what()); } } @@ -102,15 +98,11 @@ struct AsyncHelloWorker : Napi::AsyncWorker // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override - { + void OnOK() override { Napi::HandleScope scope(Env()); - if (buffer_) - { + if (buffer_) { Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); - } - else - { + } else { Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } @@ -123,8 +115,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // helloAsync is a "standalone function" because it's not a class. // If this function was not defined within a namespace ("standalone_async" // specified above), it would be in the global scope. -Napi::Value helloAsync(Napi::CallbackInfo const& info) -{ +Napi::Value helloAsync(Napi::CallbackInfo const& info) { bool louder = false; bool buffer = false; @@ -133,8 +124,7 @@ Napi::Value helloAsync(Napi::CallbackInfo const& info) // instead of throwing. // Also, "info" comes from the NAN_METHOD macro, which returns differently // according to the version of node - if (!info[1].IsFunction()) - { + if (!info[1].IsFunction()) { Napi::TypeError::New(info.Env(), "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return info.Env().Null(); } @@ -142,30 +132,25 @@ Napi::Value helloAsync(Napi::CallbackInfo const& info) Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0].IsObject()) - { + if (!info[0].IsObject()) { return utils::CallbackError("first arg 'options' must be an object", info); } Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options.Has(Napi::String::New(info.Env(), "louder"))) - { + if (options.Has(Napi::String::New(info.Env(), "louder"))) { Napi::Value louder_val = options.Get(Napi::String::New(info.Env(), "louder")); - if (!louder_val.IsBoolean()) - { + if (!louder_val.IsBoolean()) { return utils::CallbackError("option 'louder' must be a boolean", info); } louder = louder_val.As().Value(); } // Check options object for the "buffer" property, which should be a boolean // value - if (options.Has(Napi::String::New(info.Env(), "buffer"))) - { + if (options.Has(Napi::String::New(info.Env(), "buffer"))) { Napi::Value buffer_val = options.Get(Napi::String::New(info.Env(), "buffer")); - if (!buffer_val.IsBoolean()) - { + if (!buffer_val.IsBoolean()) { return utils::CallbackError("option 'buffer' must be a boolean", info); } buffer = buffer_val.As().Value(); @@ -177,7 +162,7 @@ Napi::Value helloAsync(Napi::CallbackInfo const& info) // pointer automatically. // - Napi::AsyncQueueWorker takes a pointer to a Napi::AsyncWorker and deletes // the pointer automatically. - auto * worker = new AsyncHelloWorker{louder, buffer, callback}; + auto* worker = new AsyncHelloWorker{louder, buffer, callback}; worker->Queue(); return info.Env().Undefined(); } From 54e1cac25c86ecc73688a639196f193976a2203b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 17:27:22 +0000 Subject: [PATCH 03/55] default init `name_` --- src/object_sync/hello.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index 9793fb7..9cf6b01 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -18,6 +18,6 @@ class HelloObject : public Napi::ObjectWrap { private: static Napi::FunctionReference constructor; - std::string name_; + std::string name_{}; }; } // namespace object_sync From 8a5fe79c4d1b2f9868230178b85f6cdc0610ae22 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 17:39:02 +0000 Subject: [PATCH 04/55] default construct `name_` in initialiser list to address : ``` ../src/object_async/hello_async.cpp:137:1: error: 'object_async::HelloObjectAsync::name_' should be initialized in the member initialization list [-Werror=effc++] HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) ``` changes. Lines starting --- src/object_sync/hello.cpp | 3 ++- src/object_sync/hello.hpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index 7b52814..be09944 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -31,7 +31,8 @@ Napi::FunctionReference HelloObject::constructor; // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) { + : Napi::ObjectWrap(info), + name_{} { Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index 9cf6b01..9793fb7 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -18,6 +18,6 @@ class HelloObject : public Napi::ObjectWrap { private: static Napi::FunctionReference constructor; - std::string name_{}; + std::string name_; }; } // namespace object_sync From f762f77d1ae45363143ab0651f1ce9eeb4260e9b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 17:42:13 +0000 Subject: [PATCH 05/55] clang-format --- src/object_sync/hello.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index be09944..5bebfd4 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -32,7 +32,7 @@ Napi::FunctionReference HelloObject::constructor; // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) : Napi::ObjectWrap(info), - name_{} { + name_{} { Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); From da9be9340435ca6e3069f0020c14ae09429671b8 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 16 Nov 2018 18:03:48 +0000 Subject: [PATCH 06/55] mode clang-format --- src/object_async/hello_async.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 9caaa79..21516ef 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -135,7 +135,8 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin Napi::FunctionReference HelloObjectAsync::constructor; HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) { + : Napi::ObjectWrap(info), + name_{} { Napi::Env env = info.Env(); Napi::HandleScope scope(env); From 1c8d735b9ad2d6e2608dd18f1445af0fb2218a45 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 09:35:06 +0000 Subject: [PATCH 07/55] remove extra init (tidy) --- src/object_async/hello_async.cpp | 3 +-- src/object_sync/hello.cpp | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 21516ef..9caaa79 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -135,8 +135,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin Napi::FunctionReference HelloObjectAsync::constructor; HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info), - name_{} { + : Napi::ObjectWrap(info) { Napi::Env env = info.Env(); Napi::HandleScope scope(env); diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index 5bebfd4..7b52814 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -31,8 +31,7 @@ Napi::FunctionReference HelloObject::constructor; // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info), - name_{} { + : Napi::ObjectWrap(info) { Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); From 5727b027e3ec89981b9ea4cd7c634c1a1a347964 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 09:58:07 +0000 Subject: [PATCH 08/55] make ctor explicit --- src/object_async/hello_async.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object_async/hello_async.hpp b/src/object_async/hello_async.hpp index fcbcd0f..0fef3ff 100644 --- a/src/object_async/hello_async.hpp +++ b/src/object_async/hello_async.hpp @@ -14,7 +14,7 @@ class HelloObjectAsync : public Napi::ObjectWrap { public: // initializer static Napi::Object Init(Napi::Env env, Napi::Object exports); - HelloObjectAsync(Napi::CallbackInfo const& info); + explicit HelloObjectAsync(Napi::CallbackInfo const& info); Napi::Value helloAsync(Napi::CallbackInfo const& info); private: From 85d952b1fb413d62dd19c5c0906ba12d2540848b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 09:58:40 +0000 Subject: [PATCH 09/55] pass Function by const-ref --- src/object_async/hello_async.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 9caaa79..025ebb4 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -88,7 +88,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin AsyncHelloWorker(bool louder, bool buffer, std::string const* name, - Napi::Function& cb) + Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer), @@ -157,7 +157,8 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { // ???? Napi::Env env = info.Env(); - if (info.Length() != 2 || !info[1].IsFunction()) { + if (info.Length() != 2 || !info[1].IsFunction()) + { Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return env.Null(); } From 6219f174d110f074b028a890bab6d5e8361962e7 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 10:35:27 +0000 Subject: [PATCH 10/55] annotate with NOLINT + fix passing Function by const-ref in standalone async. --- src/object_async/hello_async.cpp | 8 ++++---- src/object_sync/hello.cpp | 2 +- src/standalone_async/hello_async.cpp | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 025ebb4..c225d39 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -132,7 +132,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin std::string const* name_; }; -Napi::FunctionReference HelloObjectAsync::constructor; +Napi::FunctionReference HelloObjectAsync::constructor; // NOLINT HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) : Napi::ObjectWrap(info) { @@ -157,7 +157,7 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { // ???? Napi::Env env = info.Env(); - if (info.Length() != 2 || !info[1].IsFunction()) + if (!(info.Length() == 2 && info[1].IsFunction())) { Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return env.Null(); @@ -190,9 +190,9 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { buffer = buffer_val.As().Value(); } - auto* worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; + auto * worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; // NOLINT worker->Queue(); - return info.Env().Undefined(); + return info.Env().Undefined(); // NOLINT } Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) { diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index 7b52814..4e93568 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -27,7 +27,7 @@ // clearly organize your application. namespace object_sync { -Napi::FunctionReference HelloObject::constructor; +Napi::FunctionReference HelloObject::constructor; // NOLINT // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 038d394..16055bf 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -70,10 +70,11 @@ std::unique_ptr do_expensive_work(bool louder) { // them alive until done. // Nan AsyncWorker docs: // https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Napi::AsyncWorker { +struct AsyncHelloWorker : Napi::AsyncWorker +{ using Base = Napi::AsyncWorker; - AsyncHelloWorker(bool louder, bool buffer, Napi::Function& cb) + AsyncHelloWorker(bool louder, bool buffer, Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer) {} @@ -162,9 +163,9 @@ Napi::Value helloAsync(Napi::CallbackInfo const& info) { // pointer automatically. // - Napi::AsyncQueueWorker takes a pointer to a Napi::AsyncWorker and deletes // the pointer automatically. - auto* worker = new AsyncHelloWorker{louder, buffer, callback}; + auto* worker = new AsyncHelloWorker{louder, buffer, callback}; // NOLINT worker->Queue(); - return info.Env().Undefined(); + return info.Env().Undefined(); // NOLINT } } // namespace standalone_async From 45da8d2678047976127c44938237db33b22d590a Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 10:51:06 +0000 Subject: [PATCH 11/55] clang-format --- src/object_async/hello_async.cpp | 5 ++--- src/standalone_async/hello_async.cpp | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index c225d39..81a1970 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -157,8 +157,7 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { // ???? Napi::Env env = info.Env(); - if (!(info.Length() == 2 && info[1].IsFunction())) - { + if (!(info.Length() == 2 && info[1].IsFunction())) { Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return env.Null(); } @@ -190,7 +189,7 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { buffer = buffer_val.As().Value(); } - auto * worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; // NOLINT + auto* worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; // NOLINT worker->Queue(); return info.Env().Undefined(); // NOLINT } diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 16055bf..f8dabd0 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -70,8 +70,7 @@ std::unique_ptr do_expensive_work(bool louder) { // them alive until done. // Nan AsyncWorker docs: // https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Napi::AsyncWorker -{ +struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; AsyncHelloWorker(bool louder, bool buffer, Napi::Function const& cb) From 9843b9aba32b66d1eef528870f7a0b6260b2f1b4 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 19 Nov 2018 11:01:33 +0000 Subject: [PATCH 12/55] default 'in-class' init member variable `name_` to placate g++ --- src/object_async/hello_async.hpp | 2 +- src/object_sync/hello.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/object_async/hello_async.hpp b/src/object_async/hello_async.hpp index 0fef3ff..1b11148 100644 --- a/src/object_async/hello_async.hpp +++ b/src/object_async/hello_async.hpp @@ -21,6 +21,6 @@ class HelloObjectAsync : public Napi::ObjectWrap { // member variable // specific to each instance of the class static Napi::FunctionReference constructor; - std::string name_; + std::string name_ = ""; }; } // namespace object_async diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index 9793fb7..b99a730 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -13,11 +13,11 @@ class HelloObject : public Napi::ObjectWrap { public: // initializers static Napi::Object Init(Napi::Env env, Napi::Object exports); - HelloObject(Napi::CallbackInfo const& info); + explicit HelloObject(Napi::CallbackInfo const& info); Napi::Value hello(Napi::CallbackInfo const& info); private: static Napi::FunctionReference constructor; - std::string name_; + std::string name_ = ""; }; } // namespace object_sync From 4df69fc6b12ac3ea5da4164777f43b55a6d3b351 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 21 Nov 2018 13:24:02 +0000 Subject: [PATCH 13/55] remove LTO flags on linux builds --- common.gypi | 3 --- 1 file changed, 3 deletions(-) diff --git a/common.gypi b/common.gypi index f22ec3e..c752b17 100644 --- a/common.gypi +++ b/common.gypi @@ -38,11 +38,8 @@ 'NDEBUG' ], 'cflags': [ - '-flto' ], 'ldflags': [ - '-flto', - '-fuse-ld=<(module_root_dir)/mason_packages/.link/bin/ld' ], 'xcode_settings': { 'OTHER_CPLUSPLUSFLAGS!': [ From ac12e378b8ad88cf614043b61fc53f35f08a5185 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 21 Nov 2018 14:31:27 +0000 Subject: [PATCH 14/55] Revert "remove LTO flags on linux builds" - doh! This reverts commit 4df69fc6b12ac3ea5da4164777f43b55a6d3b351. --- common.gypi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common.gypi b/common.gypi index c752b17..f22ec3e 100644 --- a/common.gypi +++ b/common.gypi @@ -38,8 +38,11 @@ 'NDEBUG' ], 'cflags': [ + '-flto' ], 'ldflags': [ + '-flto', + '-fuse-ld=<(module_root_dir)/mason_packages/.link/bin/ld' ], 'xcode_settings': { 'OTHER_CPLUSPLUSFLAGS!': [ From bfe8b4c2819f3b28afa1b2c56d6010593b5f80b6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 26 Nov 2018 11:03:18 +0000 Subject: [PATCH 15/55] fix .clang-format --- .clang-format | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.clang-format b/.clang-format index 195c9c1..12002c4 100644 --- a/.clang-format +++ b/.clang-format @@ -33,7 +33,7 @@ BraceWrapping: BeforeElse: true IndentBraces: false BreakBeforeBinaryOperators: None -BreakBeforeBraces: Attach +BreakBeforeBraces: Custom BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: false ColumnLimit: 0 @@ -72,7 +72,7 @@ PenaltyExcessCharacter: 1000000 PenaltyReturnTypeOnItsOwnLine: 60 PointerAlignment: Left ReflowComments: true -SortIncludes: true +SortIncludes: false SpaceAfterCStyleCast: false SpaceBeforeAssignmentOperators: true SpaceBeforeParens: ControlStatements From 27b9d1db21d053cc44e388057d54639622c8ace4 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 26 Nov 2018 11:05:04 +0000 Subject: [PATCH 16/55] reimplement CPU intensive task and factor out into separate header + formatting + use `override final` --- src/cpu_intensive_task.hpp | 21 ++++++ src/module.cpp | 3 +- src/module_utils.hpp | 6 +- src/object_async/hello_async.cpp | 103 +++++++++++---------------- src/object_async/hello_async.hpp | 3 +- src/object_sync/hello.cpp | 15 ++-- src/object_sync/hello.hpp | 3 +- src/standalone/hello.cpp | 3 +- src/standalone_async/hello_async.cpp | 92 +++++++++--------------- 9 files changed, 119 insertions(+), 130 deletions(-) create mode 100644 src/cpu_intensive_task.hpp diff --git a/src/cpu_intensive_task.hpp b/src/cpu_intensive_task.hpp new file mode 100644 index 0000000..ea61a8b --- /dev/null +++ b/src/cpu_intensive_task.hpp @@ -0,0 +1,21 @@ +#pragma once + +#include +#include +#include + +namespace detail { + +// simulate CPU intensive task +inline std::unique_ptr do_expensive_work(std::string const& name, bool louder) +{ + std::this_thread::sleep_for(std::chrono::seconds(1)); + std::unique_ptr result = std::make_unique("...threads are busy async bees...hello " + name); + if (louder) + { + *result += "!!!!"; + } + return result; +} + +} // namespace detail diff --git a/src/module.cpp b/src/module.cpp index 413d8c8..d7043ff 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -8,7 +8,8 @@ // "target" is a magic var that NAN_MODULE_INIT passes into a module's scope. // When you write things to target, they become available to call from // Javascript world. -Napi::Object init(Napi::Env env, Napi::Object exports) { +Napi::Object init(Napi::Env env, Napi::Object exports) +{ // expose hello method exports.Set(Napi::String::New(env, "hello"), Napi::Function::New(env, standalone::hello)); diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 925a768..c76430b 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -21,14 +21,16 @@ namespace utils { * context * */ -inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) { +inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) +{ Napi::Object obj = Napi::Object::New(info.Env()); obj.Set("message", message); auto func = info[info.Length() - 1].As(); return func.Call({obj}); } -inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) { +inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) +{ std::string& str = *ptr; Napi::Value val = Napi::Buffer::New(env, const_cast(str.data()), diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index f3e5d6a..e189fdc 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -1,4 +1,5 @@ #include "hello_async.hpp" +#include "../cpu_intensive_task.hpp" #include "../module_utils.hpp" #include @@ -37,50 +38,16 @@ // If this was not defined within a namespace, it would be in the global scope. namespace object_async { -// This function performs expensive allocation of std::map, querying, and string -// comparison, therefore threads are nice & busy. -// Also, notice that name is passed by reference (std::string const& name) -std::unique_ptr do_expensive_work(bool louder, std::string const& name) { - - std::map container; - std::size_t work_to_do = 100000; - - for (std::size_t i = 0; i < work_to_do; ++i) { - container.emplace(i, std::to_string(i)); - } - - for (std::size_t i = 0; i < work_to_do; ++i) { - std::string const& item = container[i]; - - if (item != std::to_string(i)) { - - // AsyncHelloWorker's Execute function will take care of this error - // and return it to js-world via callback - // Marked NOLINT to avoid clang-tidy cert-err60-cpp error which we cannot - // avoid on some linux distros where std::runtime_error is not properly - // marked noexcept. Details at https://www.securecoding.cert.org/confluence/display/cplusplus/ERR60-CPP.+Exception+objects+must+be+nothrow+copy+constructible - throw std::runtime_error("Uh oh, this should never happen"); // NOLINT - } - } - - std::unique_ptr result = std::make_unique("...threads are busy async bees...hello " + name); - - if (louder) { - *result += "!!!!"; - } - - return result; -} - struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelines-special-member-functions { - using Base = Napi::AsyncWorker; + // not copyable AsyncHelloWorker(AsyncHelloWorker const&) = delete; AsyncHelloWorker& operator=(AsyncHelloWorker const&) = delete; + // ctor AsyncHelloWorker(bool louder, bool buffer, - std::string const* name, + std::string const& name, Napi::Function const& cb) : Base(cb), louder_(louder), @@ -90,10 +57,14 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() override { - try { - result_ = do_expensive_work(louder_, *name_); - } catch (std::exception const& e) { + void Execute() override final + { + try + { + result_ = detail::do_expensive_work(name_, louder_); + } + catch (std::exception const& e) + { SetError(e.what()); } } @@ -105,11 +76,15 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override { + void OnOK() override final + { Napi::HandleScope scope(Env()); - if (buffer_) { + if (buffer_) + { Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); - } else { + } + else + { Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } @@ -122,35 +97,38 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // pointing to will be kept in scope/alive for the time while AsyncHelloWorker // is using it. If we could not guarantee this then we would need to either // copy the string or pass a shared_ptr. - std::string const* name_; + std::string const& name_; }; Napi::FunctionReference HelloObjectAsync::constructor; // NOLINT HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) { + : Napi::ObjectWrap(info) +{ Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); - if (length != 1 || !info[0].IsString()) { + if (length != 1 || !info[0].IsString()) + { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); } name_ = info[0].As().Utf8Value(); - if (name_.empty()) { + if (name_.empty()) + { Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); } } // NAN_METHOD is applicable to methods you want to expose to JS world -Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { - // ???? +Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) +{ bool louder = false; bool buffer = false; - // ???? Napi::Env env = info.Env(); - if (!(info.Length() == 2 && info[1].IsFunction())) { + if (!(info.Length() == 2 && info[1].IsFunction())) + { Napi::TypeError::New(env, "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return env.Null(); } @@ -158,37 +136,42 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0].IsObject()) { + if (!info[0].IsObject()) + { return utils::CallbackError("first arg 'options' must be an object", info); } Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options.Has(Napi::String::New(env, "louder"))) { + if (options.Has(Napi::String::New(env, "louder"))) + { Napi::Value louder_val = options.Get(Napi::String::New(env, "louder")); - if (!louder_val.IsBoolean()) { + if (!louder_val.IsBoolean()) + { return utils::CallbackError("option 'louder' must be a boolean", info); } louder = louder_val.As().Value(); } // Check options object for the "buffer" property, which should be a boolean // value - if (options.Has(Napi::String::New(env, "buffer"))) { + if (options.Has(Napi::String::New(env, "buffer"))) + { Napi::Value buffer_val = options.Get(Napi::String::New(env, "buffer")); - if (!buffer_val.IsBoolean()) { + if (!buffer_val.IsBoolean()) + { return utils::CallbackError("option 'buffer' must be a boolean", info); } buffer = buffer_val.As().Value(); } - auto* worker = new AsyncHelloWorker{louder, buffer, &name_, callback}; // NOLINT + auto* worker = new AsyncHelloWorker{louder, buffer, name_, callback}; // NOLINT worker->Queue(); return info.Env().Undefined(); // NOLINT } -Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) { - +Napi::Object HelloObjectAsync::Init(Napi::Env env, Napi::Object exports) +{ Napi::Function func = DefineClass(env, "HelloObjectAsync", {InstanceMethod("helloAsync", &HelloObjectAsync::helloAsync)}); // Create a peristent reference to the class constructor. This will allow // a function called on a class prototype and a function diff --git a/src/object_async/hello_async.hpp b/src/object_async/hello_async.hpp index 1b11148..7ef2900 100644 --- a/src/object_async/hello_async.hpp +++ b/src/object_async/hello_async.hpp @@ -10,7 +10,8 @@ namespace object_async { * Also, this class adheres to the rule of Zero because we define no custom * destructor or copy constructor */ -class HelloObjectAsync : public Napi::ObjectWrap { +class HelloObjectAsync : public Napi::ObjectWrap +{ public: // initializer static Napi::Object Init(Napi::Env env, Napi::Object exports); diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index 4e93568..09faa7e 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -31,25 +31,30 @@ Napi::FunctionReference HelloObject::constructor; // NOLINT // Triggered from Javascript world when calling "new HelloObject(name)" HelloObject::HelloObject(Napi::CallbackInfo const& info) - : Napi::ObjectWrap(info) { + : Napi::ObjectWrap(info) +{ Napi::Env env = info.Env(); Napi::HandleScope scope(env); std::size_t length = info.Length(); - if (length != 1 || !info[0].IsString()) { + if (length != 1 || !info[0].IsString()) + { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); } name_ = info[0].As().Utf8Value(); - if (name_.empty()) { + if (name_.empty()) + { Napi::TypeError::New(env, "arg must be a non-empty string").ThrowAsJavaScriptException(); } } -Napi::Value HelloObject::hello(Napi::CallbackInfo const& info) { +Napi::Value HelloObject::hello(Napi::CallbackInfo const& info) +{ Napi::Env env = info.Env(); return Napi::String::New(env, name_); } -Napi::Object HelloObject::Init(Napi::Env env, Napi::Object exports) { +Napi::Object HelloObject::Init(Napi::Env env, Napi::Object exports) +{ Napi::Function func = DefineClass(env, "HelloObject", {InstanceMethod("helloMethod", &HelloObject::hello)}); // Create a peristent reference to the class constructor. This will allow diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index b99a730..bc77c19 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -9,7 +9,8 @@ namespace object_sync { * This is in a header file so we can access it across other .cpp files if necessary * Also, this class adheres to the rule of Zero because we define no custom destructor or copy constructor */ -class HelloObject : public Napi::ObjectWrap { +class HelloObject : public Napi::ObjectWrap +{ public: // initializers static Napi::Object Init(Napi::Env env, Napi::Object exports); diff --git a/src/standalone/hello.cpp b/src/standalone/hello.cpp index 7024295..b3e95b1 100644 --- a/src/standalone/hello.cpp +++ b/src/standalone/hello.cpp @@ -19,7 +19,8 @@ namespace standalone { // hello is a "standalone function" because it's not a class. // If this function was not defined within a namespace, it would be in the // global scope. -Napi::Value hello(Napi::CallbackInfo const& info) { +Napi::Value hello(Napi::CallbackInfo const& info) +{ // "info" comes from the NAN_METHOD macro, which returns differently // according to the version of node diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index f8dabd0..9bbae98 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -1,4 +1,5 @@ #include "hello_async.hpp" +#include "../cpu_intensive_task.hpp" #include "../module_utils.hpp" #include @@ -24,53 +25,16 @@ * }); */ -// If this was not defined within a namespace, it would be in the global scope. -// Namespaces are used because C++ has no notion of scoped modules, so all of -// the code you write in any file could conflict with other code. -// Namespaces are generally a great idea in C++ because it helps scale and -// clearly organize your application. namespace standalone_async { -// Expensive allocation of std::map, querying, and string comparison, -// therefore threads are busy -std::unique_ptr do_expensive_work(bool louder) { - - std::map container; - std::size_t work_to_do = 100000; - - for (std::size_t i = 0; i < work_to_do; ++i) { - container.emplace(i, std::to_string(i)); - } - - for (std::size_t i = 0; i < work_to_do; ++i) { - std::string const& item = container[i]; - if (item != std::to_string(i)) { - - // AsyncHelloWorker's Execute function will take care of this error - // and return it to js-world via callback - // Marked NOLINT to avoid clang-tidy cert-err60-cpp error which we cannot - // avoid on some linux distros where std::runtime_error is not properly - // marked noexcept. Details at https://www.securecoding.cert.org/confluence/display/cplusplus/ERR60-CPP.+Exception+objects+must+be+nothrow+copy+constructible - throw std::runtime_error("Uh oh, this should never happen"); // NOLINT - } - } - - std::unique_ptr result = std::make_unique("...threads are busy async bees...hello world"); - - if (louder) { - *result += "!!!!"; - } - - return result; -} - // This is the worker running asynchronously and calling a user-provided // callback when done. // Consider storing all C++ objects you need by value or by shared_ptr to keep // them alive until done. // Nan AsyncWorker docs: // https://github.com/nodejs/nan/blob/master/doc/asyncworker.md -struct AsyncHelloWorker : Napi::AsyncWorker { +struct AsyncHelloWorker : Napi::AsyncWorker +{ using Base = Napi::AsyncWorker; AsyncHelloWorker(bool louder, bool buffer, Napi::Function const& cb) @@ -81,12 +45,16 @@ struct AsyncHelloWorker : Napi::AsyncWorker { // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() override { + void Execute() override final + { // The try/catch is critical here: if code was added that could throw an // unhandled error INSIDE the threadpool, it would be disasterous - try { - result_ = do_expensive_work(louder_); - } catch (std::exception const& e) { + try + { + result_ = detail::do_expensive_work("world", louder_); + } + catch (std::exception const& e) + { SetError(e.what()); } } @@ -98,11 +66,15 @@ struct AsyncHelloWorker : Napi::AsyncWorker { // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override { + void OnOK() override final + { Napi::HandleScope scope(Env()); - if (buffer_) { + if (buffer_) + { Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); - } else { + } + else + { Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); } } @@ -115,16 +87,14 @@ struct AsyncHelloWorker : Napi::AsyncWorker { // helloAsync is a "standalone function" because it's not a class. // If this function was not defined within a namespace ("standalone_async" // specified above), it would be in the global scope. -Napi::Value helloAsync(Napi::CallbackInfo const& info) { +Napi::Value helloAsync(Napi::CallbackInfo const& info) +{ bool louder = false; bool buffer = false; // Check second argument, should be a 'callback' function. - // This allows us to set the callback so we can use it to return errors - // instead of throwing. - // Also, "info" comes from the NAN_METHOD macro, which returns differently - // according to the version of node - if (!info[1].IsFunction()) { + if (!info[1].IsFunction()) + { Napi::TypeError::New(info.Env(), "second arg 'callback' must be a function").ThrowAsJavaScriptException(); return info.Env().Null(); } @@ -132,25 +102,29 @@ Napi::Value helloAsync(Napi::CallbackInfo const& info) { Napi::Function callback = info[1].As(); // Check first argument, should be an 'options' object - if (!info[0].IsObject()) { + if (!info[0].IsObject()) + { return utils::CallbackError("first arg 'options' must be an object", info); } Napi::Object options = info[0].As(); // Check options object for the "louder" property, which should be a boolean // value - if (options.Has(Napi::String::New(info.Env(), "louder"))) { + if (options.Has(Napi::String::New(info.Env(), "louder"))) + { Napi::Value louder_val = options.Get(Napi::String::New(info.Env(), "louder")); - if (!louder_val.IsBoolean()) { + if (!louder_val.IsBoolean()) + { return utils::CallbackError("option 'louder' must be a boolean", info); } louder = louder_val.As().Value(); } - // Check options object for the "buffer" property, which should be a boolean - // value - if (options.Has(Napi::String::New(info.Env(), "buffer"))) { + // Check options object for the "buffer" property, which should be a boolean value + if (options.Has(Napi::String::New(info.Env(), "buffer"))) + { Napi::Value buffer_val = options.Get(Napi::String::New(info.Env(), "buffer")); - if (!buffer_val.IsBoolean()) { + if (!buffer_val.IsBoolean()) + { return utils::CallbackError("option 'buffer' must be a boolean", info); } buffer = buffer_val.As().Value(); From ae1df1d6818e38beb9d6465d54567617a80b6850 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 26 Nov 2018 11:30:09 +0000 Subject: [PATCH 17/55] more cleaning up --- src/module.cpp | 13 ++--------- src/module_utils.hpp | 13 ++++------- src/object_sync/hello.hpp | 8 +++---- src/standalone/hello.cpp | 12 +---------- src/standalone/hello.hpp | 32 ++-------------------------- src/standalone_async/hello_async.hpp | 29 ++----------------------- 6 files changed, 15 insertions(+), 92 deletions(-) diff --git a/src/module.cpp b/src/module.cpp index d7043ff..f01d540 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -5,12 +5,8 @@ #include // #include "your_code.hpp" -// "target" is a magic var that NAN_MODULE_INIT passes into a module's scope. -// When you write things to target, they become available to call from -// Javascript world. Napi::Object init(Napi::Env env, Napi::Object exports) { - // expose hello method exports.Set(Napi::String::New(env, "hello"), Napi::Function::New(env, standalone::hello)); @@ -38,13 +34,8 @@ Napi::Object init(Napi::Env env, Napi::Object exports) // Include your .hpp file at the top of this file. } -// Here we initialize the module (we only do this once) -// by attaching the init function to the module. This invokes -// a variety of magic from inside nodejs core that we don't need to -// worry about, but if you care the details are at https://github.com/nodejs/node/blob/34d1b1144e1af8382dad71c28c8d956ebf709801/src/node.h#L431-L518 -// We mark this NOLINT to avoid the clang-tidy checks -// warning about code inside nodejs that we don't control and can't -// directly change to avoid the warning. +// Initialize the module (we only do this once) +// Mark this NOLINT to avoid the clang-tidy checks // NODE_GYP_MODULE_NAME is the name of our module as defined in 'target_name' // variable in the 'binding.gyp', which is passed along as a compiler define NODE_API_MODULE(NODE_GYP_MODULE_NAME, init) // NOLINT diff --git a/src/module_utils.hpp b/src/module_utils.hpp index c76430b..77c7f6c 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -10,15 +10,8 @@ namespace utils { * throwing errors. * Usage: * -* Napi::Function callback; -* return CallbackError("error message", callback); // "return" is important to -* prevent duplicate callbacks from being fired! -* -* -* "inline" is important here as well. See for more contex: -* - https://github.com/mapbox/cpp/blob/master/glossary.md#inline-keyword -* - https://github.com/mapbox/node-cpp-skel/pull/52#discussion_r126847394 for -* context +* Napi::CallbackInfo info; +* return CallbackError("error message", info); * */ inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo const& info) @@ -26,6 +19,8 @@ inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo Napi::Object obj = Napi::Object::New(info.Env()); obj.Set("message", message); auto func = info[info.Length() - 1].As(); + // ^^^ here we assume that info has a valid callback function + // TODO: consider changing either method signature or adding internal checks return func.Call({obj}); } diff --git a/src/object_sync/hello.hpp b/src/object_sync/hello.hpp index bc77c19..255992b 100644 --- a/src/object_sync/hello.hpp +++ b/src/object_sync/hello.hpp @@ -5,10 +5,10 @@ namespace object_sync { /** - * HelloObject class - * This is in a header file so we can access it across other .cpp files if necessary - * Also, this class adheres to the rule of Zero because we define no custom destructor or copy constructor - */ + * HelloObject class + * This is in a header file so we can access it across other .cpp files if necessary + * Also, this class adheres to the rule of Zero because we define no custom destructor or copy constructor + */ class HelloObject : public Napi::ObjectWrap { public: diff --git a/src/standalone/hello.cpp b/src/standalone/hello.cpp index b3e95b1..ce4be7a 100644 --- a/src/standalone/hello.cpp +++ b/src/standalone/hello.cpp @@ -10,21 +10,11 @@ * console.log(check); // => "hello world" */ namespace standalone { -// If this was not defined within a namespace, it would be in the global scope. -// Namespaces are used because C++ has no notion of scoped modules, so all of -// the code you write in any file could conflict with other code. -// Namespaces are generally a great idea in C++ because it helps scale and -// clearly organize your application. -// hello is a "standalone function" because it's not a class. -// If this function was not defined within a namespace, it would be in the -// global scope. Napi::Value hello(Napi::CallbackInfo const& info) { - - // "info" comes from the NAN_METHOD macro, which returns differently - // according to the version of node Napi::Env env = info.Env(); return Napi::String::New(env, "hello world"); } + } // namespace standalone diff --git a/src/standalone/hello.hpp b/src/standalone/hello.hpp index d461cf1..08d7498 100644 --- a/src/standalone/hello.hpp +++ b/src/standalone/hello.hpp @@ -1,37 +1,9 @@ #pragma once #include -// specify #include with carrots, ex: --> look for header in global -// specify #include with quotes, ex: "hello.hpp" --> look for header in location -// relative to this file - -/* - - Namespace is an organizational method that helps to clearly show where a - method is coming from. - Namespaces are generally a great idea in C++ because they help us scale - things. C++ has no notion of scoped modules, - so all of the code you write in any file could potentially conflict with other - classes/functions/etc. - Namespaces help to differentiate pieces of your code. - - The convention In this skeleton is to name the namespace to match the name of - the subdirectory where it lives. - So in this case, the namespace is called "standalone" because this method - lives within the "standalone" subdirectory. - If there is another "hello" function used for another example, the compiler - will know the difference between the two: - - standalone::hello - - VS - - potato::hello - -*/ namespace standalone { -// hello, custom sync method tied to module.cpp -// method's logic lives in hello.cpp +// hello, custom sync method Napi::Value hello(Napi::CallbackInfo const& info); + } // namespace standalone diff --git a/src/standalone_async/hello_async.hpp b/src/standalone_async/hello_async.hpp index 6e1a786..ab21844 100644 --- a/src/standalone_async/hello_async.hpp +++ b/src/standalone_async/hello_async.hpp @@ -1,35 +1,10 @@ #pragma once #include -// carrots, ex: --> look for header in global -// quotes, ex: "hello.hpp" --> look for header in location relative to this file -/* - - Namespace is an organizational method that helps to clearly show where a - method is coming from. - Namespaces are generally a great idea in C++ because they help us scale - things. C++ has no notion of scoped modules, - so all of the code you write in any file could potentially conflict with other - classes/functions/etc. - Namespaces help to differentiate pieces of your code. - - The convention In this skeleton is to name the namespace to match the name of - the subdirectory where it lives. - So in this case, the namespace is called "standalone" because this method - lives within the "standalone" subdirectory. - If there is another "hello" function used for another example, the compiler - will know the difference between the two: - - standalone::hello - - VS - - potato::hello - -*/ namespace standalone_async { -// hello, custom sync method tied to module.cpp +// hello, custom sync method // method's logic lives in hello.cpp Napi::Value helloAsync(Napi::CallbackInfo const& info); + } // namespace standalone_async From f56151fc9adef4f6dbb1b8d967ea4cdb4245a349 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 26 Nov 2018 13:40:13 +0000 Subject: [PATCH 18/55] remove redundant `override` c++11 --- src/object_async/hello_async.cpp | 4 ++-- src/standalone_async/hello_async.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index e189fdc..10bf2fb 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -57,7 +57,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() override final + void Execute() final { try { @@ -76,7 +76,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override final + void OnOK() final { Napi::HandleScope scope(Env()); if (buffer_) diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 9bbae98..b844020 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -45,7 +45,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() override final + void Execute() final { // The try/catch is critical here: if code was added that could throw an // unhandled error INSIDE the threadpool, it would be disasterous @@ -66,7 +66,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() override final + void OnOK() final { Napi::HandleScope scope(Env()); if (buffer_) From 8aa7626fb43ed884dafa3f104eb2ea5a84a1ac6e Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 26 Mar 2020 10:09:36 +0000 Subject: [PATCH 19/55] fix comment --- src/object_async/hello_async.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 10bf2fb..a3393a7 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -92,11 +92,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin std::unique_ptr result_ = std::make_unique(); bool const louder_; bool const buffer_; - // We use a pointer here to avoid copying the string data. - // This works because we know that the original string we are - // pointing to will be kept in scope/alive for the time while AsyncHelloWorker - // is using it. If we could not guarantee this then we would need to either - // copy the string or pass a shared_ptr. + // Store `name_` by const reference to avoid copying std::string const& name_; }; From b95d393f41d53a17b8d6db2e71e28368fe3ca5f1 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 27 Mar 2020 10:42:40 +0000 Subject: [PATCH 20/55] Make simpler --- src/module_utils.hpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 77c7f6c..08e1b90 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -24,16 +24,15 @@ inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo return func.Call({obj}); } -inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& ptr) +inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& str_p) { - std::string& str = *ptr; Napi::Value val = Napi::Buffer::New(env, - const_cast(str.data()), - str.size(), + const_cast(str_p->data()), + str_p->size(), [](Napi::Env, char*, std::string* s) { delete s; }, - ptr.release()); + str_p.release()); return val; } From 6854cf2cfdc70c32ddcb8b6a35fefa27155500d6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 10:38:30 +0100 Subject: [PATCH 21/55] Update npm packages --- package-lock.json | 12 ++++++------ package.json | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index cfd755d..fc934b0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -592,9 +592,9 @@ } }, "lodash": { - "version": "4.17.10", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.10.tgz", - "integrity": "sha512-UejweD1pDoXu+AD825lWwp4ZGtSwgnpZxb3JDViD7StjQz+Nb/6l093lx4OQ0foGWNRoc19mWy7BzL+UAK2iVg==", + "version": "4.17.15", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", + "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==", "dev": true }, "mason-js-sdk": { @@ -635,9 +635,9 @@ } }, "minimist": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", - "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=" + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" }, "minipass": { "version": "2.3.3", diff --git a/package.json b/package.json index 362bfdb..85f59e5 100644 --- a/package.json +++ b/package.json @@ -22,10 +22,10 @@ }, "devDependencies": { "aws-sdk": "^2.4.7", - "tape": "^4.5.1", + "bytes": "^2.4.0", "d3-queue": "^3.0.1", - "minimist": "~1.2.0", - "bytes": "^2.4.0" + "minimist": "^1.2.5", + "tape": "^4.5.1" }, "binary": { "module_name": "module", From b914a10685f75e498622aed5475395f758944d79 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 11:03:41 +0100 Subject: [PATCH 22/55] add link to docs --- src/module_utils.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 08e1b90..3e9acd6 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -33,6 +33,8 @@ inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr Date: Mon, 30 Mar 2020 11:03:50 +0100 Subject: [PATCH 23/55] Remove Nan references + update link --- src/object_async/hello_async.cpp | 1 - src/standalone_async/hello_async.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index a3393a7..d57c994 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -116,7 +116,6 @@ HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) } } -// NAN_METHOD is applicable to methods you want to expose to JS world Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) { bool louder = false; diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index b844020..db501ec 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -31,8 +31,8 @@ namespace standalone_async { // callback when done. // Consider storing all C++ objects you need by value or by shared_ptr to keep // them alive until done. -// Nan AsyncWorker docs: -// https://github.com/nodejs/nan/blob/master/doc/asyncworker.md +// Napi::AsyncWorker docs: +// https://github.com/nodejs/node-addon-api/blob/master/doc/async_worker.md struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; From 17de6e2637057aeac0b1897049bdaf61d7200318 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 11:12:50 +0100 Subject: [PATCH 24/55] remove node-6 builds --- .travis.yml | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8447789..63533d6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,11 +2,10 @@ language: node_js sudo: false -# enable c++11/14 builds addons: apt: sources: [ 'ubuntu-toolchain-r-test' ] - packages: [ 'libstdc++-4.9-dev' ] + packages: [ 'libstdc++-5-dev' ] install: - node -v @@ -35,7 +34,7 @@ matrix: ## ** Builds that are published ** - # linux cfi build node v6/release + # linux cfi build node v10/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" node_js: 10 @@ -55,14 +54,6 @@ matrix: - os: linux env: BUILDTYPE=debug node_js: 8 - # linux publishable node v6/release - - os: linux - env: BUILDTYPE=release - node_js: 6 - # linux publishable node v6/debug - - os: linux - env: BUILDTYPE=debug - node_js: 6 # osx publishable node v10/release - os: osx osx_image: xcode9.3 @@ -73,17 +64,7 @@ matrix: osx_image: xcode9.3 env: BUILDTYPE=release node_js: 8 - # osx publishable node v6/release - - os: osx - osx_image: xcode9.3 - env: BUILDTYPE=release - node_js: 6 - # osx publishable node v6/debug - - os: osx - osx_image: xcode9.3 - env: BUILDTYPE=debug - node_js: 6 - # linux sanitizer build node v6/debug + # linux sanitizer build node v10/debug - os: linux env: BUILDTYPE=debug TOOLSET=asan node_js: 10 @@ -93,7 +74,7 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: - # osx sanitizer build node v6/debug + # osx sanitizer build node v10/debug - os: osx env: BUILDTYPE=debug TOOLSET=asan node_js: 10 From 6258ccf7f5edae4f9456d8b2aca345b33c11df19 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 14:55:31 +0100 Subject: [PATCH 25/55] Allocate new Buffer and copy data into it. --- src/module_utils.hpp | 15 --------------- src/object_async/hello_async.cpp | 2 +- src/standalone_async/hello_async.cpp | 2 +- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 3e9acd6..48c8489 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -23,19 +23,4 @@ inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo // TODO: consider changing either method signature or adding internal checks return func.Call({obj}); } - -inline Napi::Value NewBufferFrom(Napi::Env const& env, std::unique_ptr&& str_p) -{ - Napi::Value val = Napi::Buffer::New(env, - const_cast(str_p->data()), - str_p->size(), - [](Napi::Env, char*, std::string* s) { - delete s; - }, - str_p.release()); - // ^^^ New Bufer does not assume ownership for the data and expects it to be valid for the lifetime of the object. - // Docs: https://github.com/nodejs/node-addon-api/blob/master/doc/buffer.md - return val; -} - } // namespace utils diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index d57c994..9b49119 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -81,7 +81,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin Napi::HandleScope scope(Env()); if (buffer_) { - Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); + Callback().Call({Env().Null(), Napi::Buffer::Copy(Env(), result_->data(), result_->size())}); } else { diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index db501ec..8f091ee 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -71,7 +71,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker Napi::HandleScope scope(Env()); if (buffer_) { - Callback().Call({Env().Null(), utils::NewBufferFrom(Env(), std::move(result_))}); + Callback().Call({Env().Null(), Napi::Buffer::Copy(Env(), result_->data(), result_->size())}); } else { From 227711923cf00a09c23b4ebad351f29252060e0b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 15:34:58 +0100 Subject: [PATCH 26/55] Add empty dtor to avoid memory leaks + make AsyncHelloWorker non-copyable --- src/object_async/hello_async.cpp | 2 ++ src/standalone_async/hello_async.cpp | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 9b49119..d9a0415 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -54,6 +54,8 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin buffer_(buffer), name_(name) {} + ~AsyncHelloWorker() {} // empty destructor + // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 8f091ee..eb1d3ad 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -36,12 +36,18 @@ namespace standalone_async { struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; - + // not copyable + AsyncHelloWorker(AsyncHelloWorker const&) = delete; + AsyncHelloWorker& operator=(AsyncHelloWorker const&) = delete; + // ctor AsyncHelloWorker(bool louder, bool buffer, Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer) {} + + ~AsyncHelloWorker() {} // empty dtor + // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. From 0e9c970d69b1b9c3d645a32c3df88b2cc1f3b187 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 15:35:55 +0100 Subject: [PATCH 27/55] clang-format --- src/standalone_async/hello_async.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index eb1d3ad..7db4817 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -45,7 +45,6 @@ struct AsyncHelloWorker : Napi::AsyncWorker louder_(louder), buffer_(buffer) {} - ~AsyncHelloWorker() {} // empty dtor // The Execute() function is getting called when the worker starts to run. From 8f37517d68b3f1a26da221f306be4d1106a26a41 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 15:52:26 +0100 Subject: [PATCH 28/55] clang-tidy --- src/object_async/hello_async.cpp | 7 +------ src/standalone_async/hello_async.cpp | 5 ----- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index d9a0415..5a1fd3f 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -38,12 +38,9 @@ // If this was not defined within a namespace, it would be in the global scope. namespace object_async { -struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelines-special-member-functions +struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; - // not copyable - AsyncHelloWorker(AsyncHelloWorker const&) = delete; - AsyncHelloWorker& operator=(AsyncHelloWorker const&) = delete; // ctor AsyncHelloWorker(bool louder, bool buffer, @@ -54,8 +51,6 @@ struct AsyncHelloWorker : Napi::AsyncWorker // NOLINT to disable cppcoreguidelin buffer_(buffer), name_(name) {} - ~AsyncHelloWorker() {} // empty destructor - // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 7db4817..575ada2 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -36,17 +36,12 @@ namespace standalone_async { struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; - // not copyable - AsyncHelloWorker(AsyncHelloWorker const&) = delete; - AsyncHelloWorker& operator=(AsyncHelloWorker const&) = delete; // ctor AsyncHelloWorker(bool louder, bool buffer, Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer) {} - ~AsyncHelloWorker() {} // empty dtor - // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. From ea29accaf348a7a9c56823c03d47aac292b0464f Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 16:19:59 +0100 Subject: [PATCH 29/55] require mode-addon-api >= 2.0.0 --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index fc934b0..5490543 100644 --- a/package-lock.json +++ b/package-lock.json @@ -694,9 +694,9 @@ } }, "node-addon-api": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-1.6.0.tgz", - "integrity": "sha512-HEUPBHfdH4CLR1Qq4/Ek8GT/qFSvpApjJQmcYdLCL51ADU/Y11kMuFAdIevhNrPh3ylqVGA8k6vI/oi4YUAHbA==" + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-2.0.0.tgz", + "integrity": "sha512-ASCL5U13as7HhOExbT6OlWJJUV/lLzL2voOSP1UVehpRD8FbSrSDjfScK/KwAvVTI5AS6r4VwbOMlIqtvRidnA==" }, "node-pre-gyp": { "version": "0.10.0", diff --git a/package.json b/package.json index 85f59e5..a2765d7 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "license": "ISC", "dependencies": { "mason-js-sdk": "~0.1.4", - "node-addon-api": "^1.5.0", + "node-addon-api": "^2.0.0", "node-pre-gyp": "~0.10.0" }, "devDependencies": { From d4e628415466a60b9107e8144577838cc62546a7 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 30 Mar 2020 17:35:54 +0100 Subject: [PATCH 30/55] Take ownership of underlying data (NOTE: clang-tidy doesn't like this:) --- src/object_async/hello_async.cpp | 9 ++++++++- src/standalone_async/hello_async.cpp | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 5a1fd3f..d7bf269 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -78,7 +78,14 @@ struct AsyncHelloWorker : Napi::AsyncWorker Napi::HandleScope scope(Env()); if (buffer_) { - Callback().Call({Env().Null(), Napi::Buffer::Copy(Env(), result_->data(), result_->size())}); + auto buffer = Napi::Buffer::New(Env(), + const_cast(result_->data()), + result_->size(), + [](Napi::Env, char*, std::string* s) { + delete s; + }, + result_.release()); + Callback().Call({Env().Null(), buffer}); } else { diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 575ada2..6789e3f 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -71,7 +71,14 @@ struct AsyncHelloWorker : Napi::AsyncWorker Napi::HandleScope scope(Env()); if (buffer_) { - Callback().Call({Env().Null(), Napi::Buffer::Copy(Env(), result_->data(), result_->size())}); + auto buffer = Napi::Buffer::New(Env(), + const_cast(result_->data()), + result_->size(), + [](Napi::Env, char*, std::string* s) { + delete s; + }, + result_.release()); + Callback().Call({Env().Null(), buffer}); } else { From 952946fa39eac51104e255fc4f8a7c6393895d4d Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 11:42:52 +0100 Subject: [PATCH 31/55] Add missing return statements --- src/object_async/hello_async.cpp | 1 + src/object_sync/hello.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index d7bf269..ea2f5a6 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -112,6 +112,7 @@ HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) if (length != 1 || !info[0].IsString()) { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); + return; } name_ = info[0].As().Utf8Value(); if (name_.empty()) diff --git a/src/object_sync/hello.cpp b/src/object_sync/hello.cpp index 09faa7e..e2a8344 100644 --- a/src/object_sync/hello.cpp +++ b/src/object_sync/hello.cpp @@ -39,6 +39,7 @@ HelloObject::HelloObject(Napi::CallbackInfo const& info) if (length != 1 || !info[0].IsString()) { Napi::TypeError::New(env, "String expected").ThrowAsJavaScriptException(); + return; } name_ = info[0].As().Utf8Value(); if (name_.empty()) From 2accc6413d80d19bc2a1b1bd284dd290b9e946a7 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 12:49:07 +0100 Subject: [PATCH 32/55] Use latest (HEAD) hode-addon-api module (fixes SIGILL crash in "Control Flow Integriry" builds (-fsanitize=cfi) with v2.0.0) --- package-lock.json | 5 ++--- package.json | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5490543..1ccdc48 100644 --- a/package-lock.json +++ b/package-lock.json @@ -694,9 +694,8 @@ } }, "node-addon-api": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-2.0.0.tgz", - "integrity": "sha512-ASCL5U13as7HhOExbT6OlWJJUV/lLzL2voOSP1UVehpRD8FbSrSDjfScK/KwAvVTI5AS6r4VwbOMlIqtvRidnA==" + "version": "github:nodejs/node-addon-api#2c3d5df4634dec4e8c41cf251e15cd87d3aabaf8", + "from": "github:nodejs/node-addon-api" }, "node-pre-gyp": { "version": "0.10.0", diff --git a/package.json b/package.json index a2765d7..014ec40 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "license": "ISC", "dependencies": { "mason-js-sdk": "~0.1.4", - "node-addon-api": "^2.0.0", + "node-addon-api": "nodejs/node-addon-api", "node-pre-gyp": "~0.10.0" }, "devDependencies": { From 79f25074e0c3f24699248f3851eabe746a6e8d98 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 12:53:21 +0100 Subject: [PATCH 33/55] Fix test - use exact error message match --- test/hello_object_async.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/hello_object_async.test.js b/test/hello_object_async.test.js index ec2400e..e2c4e64 100644 --- a/test/hello_object_async.test.js +++ b/test/hello_object_async.test.js @@ -55,7 +55,7 @@ test('error: handles non-string arg within constructor', function(t) { var H = new module.HelloObjectAsync(24); } catch(err) { console.log(err.message); - t.ok(err.message.indexOf('A string was expected') > -1, 'expected error message'); + t.equal(err.message, 'String expected', 'expected error message'); t.end(); } }); @@ -103,7 +103,7 @@ test('error: handles missing arg', function(t) { var H = new module.HelloObjectAsync(); } catch (err) { t.ok(err, 'expected error'); - t.ok(err.message.indexOf('A string was expected') > -1, 'expected error message'); + t.equal(err.message, 'String expected', 'expected error message'); t.end(); } }); From 88f37e5bc13e7002d60608e7309b781559129ba3 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 16:22:35 +0100 Subject: [PATCH 34/55] sleep for 100ms to speed up tests running time --- src/cpu_intensive_task.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu_intensive_task.hpp b/src/cpu_intensive_task.hpp index ea61a8b..2aa2325 100644 --- a/src/cpu_intensive_task.hpp +++ b/src/cpu_intensive_task.hpp @@ -9,7 +9,7 @@ namespace detail { // simulate CPU intensive task inline std::unique_ptr do_expensive_work(std::string const& name, bool louder) { - std::this_thread::sleep_for(std::chrono::seconds(1)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); std::unique_ptr result = std::make_unique("...threads are busy async bees...hello " + name); if (louder) { From 586e60fec4d96d49a51da6a110c63f033ab603a8 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 16:36:19 +0100 Subject: [PATCH 35/55] Store name_ member variable by value + clang tidy --- src/object_async/hello_async.cpp | 44 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index ea2f5a6..78ab114 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -3,10 +3,10 @@ #include "../module_utils.hpp" #include -#include #include #include #include +#include /** * Asynchronous class, called HelloObjectAsync @@ -44,17 +44,17 @@ struct AsyncHelloWorker : Napi::AsyncWorker // ctor AsyncHelloWorker(bool louder, bool buffer, - std::string const& name, + std::string name, Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer), - name_(name) {} + name_(std::move(name)) {} // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() final + void Execute() override { try { @@ -73,31 +73,33 @@ struct AsyncHelloWorker : Napi::AsyncWorker // - You have access to Javascript v8 objects again // - You have to translate from C++ member variables to Javascript v8 objects // - Finally, you call the user's callback with your results - void OnOK() final + void OnOK() override { Napi::HandleScope scope(Env()); - if (buffer_) + if (!Callback().IsEmpty()) { - auto buffer = Napi::Buffer::New(Env(), - const_cast(result_->data()), - result_->size(), - [](Napi::Env, char*, std::string* s) { - delete s; - }, - result_.release()); - Callback().Call({Env().Null(), buffer}); - } - else - { - Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + if (buffer_) + { + auto buffer = Napi::Buffer::New(Env(), + const_cast(result_->data()), + result_->size(), + [](Napi::Env, char*, std::string* s) { + delete s; + }, + result_.release()); + Callback().Call({Env().Null(), buffer}); + } + else + { + Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + } } } - std::unique_ptr result_ = std::make_unique(); + std::unique_ptr result_; bool const louder_; bool const buffer_; - // Store `name_` by const reference to avoid copying - std::string const& name_; + std::string const name_; }; Napi::FunctionReference HelloObjectAsync::constructor; // NOLINT From e7698440133af82ab9fd23a0dd5cd35b828def01 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 16:38:41 +0100 Subject: [PATCH 36/55] clang-format --- src/object_async/hello_async.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index 78ab114..cc2e03d 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -44,7 +44,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // ctor AsyncHelloWorker(bool louder, bool buffer, - std::string name, + std::string name, Napi::Function const& cb) : Base(cb), louder_(louder), From 2520acef4ee1e13a91d98e729a9b7418ea7047d6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 16:44:11 +0100 Subject: [PATCH 37/55] Initialize result_ --- src/object_async/hello_async.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index cc2e03d..a1586b8 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -96,7 +96,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker } } - std::unique_ptr result_; + std::unique_ptr result_ = nullptr; bool const louder_; bool const buffer_; std::string const name_; From cb186eb511ca53dd2200aea1f3eca90aeb03bf6e Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Tue, 31 Mar 2020 16:51:41 +0100 Subject: [PATCH 38/55] c++ style --- src/standalone_async/hello_async.cpp | 33 +++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 6789e3f..5338fe1 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -45,7 +45,7 @@ struct AsyncHelloWorker : Napi::AsyncWorker // The Execute() function is getting called when the worker starts to run. // - You only have access to member variables stored in this worker. // - You do not have access to Javascript v8 objects here. - void Execute() final + void Execute() override { // The try/catch is critical here: if code was added that could throw an // unhandled error INSIDE the threadpool, it would be disasterous @@ -69,24 +69,27 @@ struct AsyncHelloWorker : Napi::AsyncWorker void OnOK() final { Napi::HandleScope scope(Env()); - if (buffer_) + if (!Callback().IsEmpty()) { - auto buffer = Napi::Buffer::New(Env(), - const_cast(result_->data()), - result_->size(), - [](Napi::Env, char*, std::string* s) { - delete s; - }, - result_.release()); - Callback().Call({Env().Null(), buffer}); - } - else - { - Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + if (buffer_) + { + auto buffer = Napi::Buffer::New(Env(), + const_cast(result_->data()), + result_->size(), + [](Napi::Env, char*, std::string* s) { + delete s; + }, + result_.release()); + Callback().Call({Env().Null(), buffer}); + } + else + { + Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + } } } - std::unique_ptr result_ = std::make_unique(); + std::unique_ptr result_ = nullptr; const bool louder_; const bool buffer_; }; From 6d08efe1bfc4f12fd11115168082cf98e1f9e3f6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 12:43:27 +0100 Subject: [PATCH 39/55] Return std::unique_ptr> and stop abusing std::string::data with const_cast (clang-tidy) --- src/cpu_intensive_task.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cpu_intensive_task.hpp b/src/cpu_intensive_task.hpp index 2aa2325..2723f5f 100644 --- a/src/cpu_intensive_task.hpp +++ b/src/cpu_intensive_task.hpp @@ -3,17 +3,20 @@ #include #include #include +#include namespace detail { // simulate CPU intensive task -inline std::unique_ptr do_expensive_work(std::string const& name, bool louder) +inline std::unique_ptr> do_expensive_work(std::string const& name, bool louder) { std::this_thread::sleep_for(std::chrono::milliseconds(100)); - std::unique_ptr result = std::make_unique("...threads are busy async bees...hello " + name); + std::string str = "...threads are busy async bees...hello " + name; + std::unique_ptr> result = std::make_unique>(str.begin(), str.end()); if (louder) { - *result += "!!!!"; + std::string extra{"!!!!"}; + std::copy(extra.c_str(), extra.c_str() + extra.length(), back_inserter(*result)); } return result; } From 4f5883f375d048a14015241e952ab262e197f862 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 12:45:09 +0100 Subject: [PATCH 40/55] update to use std::vector + add gsl::owner implementation + explicitely transfer ownership to keep clang-tidy happy (cppcoreguidelines-owning-memory) --- src/module_utils.hpp | 7 +++++++ src/object_async/hello_async.cpp | 11 ++++++----- src/standalone_async/hello_async.cpp | 10 +++++----- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/module_utils.hpp b/src/module_utils.hpp index 48c8489..6266441 100644 --- a/src/module_utils.hpp +++ b/src/module_utils.hpp @@ -24,3 +24,10 @@ inline Napi::Value CallbackError(std::string const& message, Napi::CallbackInfo return func.Call({obj}); } } // namespace utils + +namespace gsl { +template +using owner = T; +} // namespace gsl + +// ^^^ type alias required for clang-tidy (cppcoreguidelines-owning-memory) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index a1586b8..f5d4d27 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -36,6 +36,7 @@ */ // If this was not defined within a namespace, it would be in the global scope. + namespace object_async { struct AsyncHelloWorker : Napi::AsyncWorker @@ -81,22 +82,22 @@ struct AsyncHelloWorker : Napi::AsyncWorker if (buffer_) { auto buffer = Napi::Buffer::New(Env(), - const_cast(result_->data()), + result_->data(), result_->size(), - [](Napi::Env, char*, std::string* s) { - delete s; + [](Napi::Env, char*, gsl::owner*> v) { + delete v; }, result_.release()); Callback().Call({Env().Null(), buffer}); } else { - Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + Callback().Call({Env().Null(), Napi::String::New(Env(), result_->data(), result_->size())}); } } } - std::unique_ptr result_ = nullptr; + std::unique_ptr> result_ = nullptr; bool const louder_; bool const buffer_; std::string const name_; diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 5338fe1..0514d17 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -74,22 +74,22 @@ struct AsyncHelloWorker : Napi::AsyncWorker if (buffer_) { auto buffer = Napi::Buffer::New(Env(), - const_cast(result_->data()), + result_->data(), result_->size(), - [](Napi::Env, char*, std::string* s) { - delete s; + [](Napi::Env, char*, gsl::owner*> v) { + delete v; }, result_.release()); Callback().Call({Env().Null(), buffer}); } else { - Callback().Call({Env().Null(), Napi::String::New(Env(), *result_)}); + Callback().Call({Env().Null(), Napi::String::New(Env(), result_->data(), result_->size())}); } } } - std::unique_ptr result_ = nullptr; + std::unique_ptr> result_ = nullptr; const bool louder_; const bool buffer_; }; From bd6bdb6ca1d7d3b68efc367f22e7fd00dcb4a3c8 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 12:59:16 +0100 Subject: [PATCH 41/55] Only target node v10 and v12 --- .travis.yml | 134 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 117 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 63533d6..ccb99c1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,8 @@ matrix: ## ** Builds that are published ** + # Node 10 + # linux cfi build node v10/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" @@ -46,24 +48,11 @@ matrix: - os: linux env: BUILDTYPE=debug node_js: 10 - # linux publishable node v8/release - - os: linux - env: BUILDTYPE=release - node_js: 8 - # linux publishable node v8/debug - - os: linux - env: BUILDTYPE=debug - node_js: 8 # osx publishable node v10/release - os: osx osx_image: xcode9.3 env: BUILDTYPE=release node_js: 10 - # osx publishable node v8/release - - os: osx - osx_image: xcode9.3 - env: BUILDTYPE=release - node_js: 8 # linux sanitizer build node v10/debug - os: linux env: BUILDTYPE=debug TOOLSET=asan @@ -74,6 +63,7 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: + - true # osx sanitizer build node v10/debug - os: osx env: BUILDTYPE=debug TOOLSET=asan @@ -84,11 +74,11 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: + - true - ## ** Builds that do not get published ** - - # g++ build (default builds all use clang++) - - os: linux + ## ** Builds that do not get published ** + # g++ build (default builds all use clang++) + - os: linux env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm" node_js: 10 addons: @@ -103,6 +93,7 @@ matrix: - make ${BUILDTYPE} # Overrides `script` to disable publishing script: + - true # Coverage build - os: linux env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" @@ -127,8 +118,10 @@ matrix: - make format # Overrides `before_script`, no need to run tests before_script: + - true # Overrides `script` to disable publishing script: + - true # Clang tidy build - os: linux env: CLANG_TIDY @@ -142,5 +135,112 @@ matrix: - make tidy # Overrides `before_script`, no need to run tests before_script: + - true + # Overrides `script` to disable publishing + script: + - true + + # Node 12 + # linux cfi build node v12/release + - os: linux + env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" + node_js: 12 + # linux publishable node v12/release + - os: linux + env: BUILDTYPE=release + node_js: 12 + # linux publishable node v12/debug + - os: linux + env: BUILDTYPE=debug + node_js: 12 + # osx publishable node v12/release + - os: osx + osx_image: xcode9.3 + env: BUILDTYPE=release + node_js: 12 + # linux sanitizer build node v12/debug + - os: linux + env: BUILDTYPE=debug TOOLSET=asan + node_js: 12 + sudo: required + # Overrides `install` to set up custom asan flags + install: + - make sanitize + # Overrides `before_script` (tests are already run in `make sanitize`) + before_script: + - true + # osx sanitizer build node v12/debug + - os: osx + env: BUILDTYPE=debug TOOLSET=asan + node_js: 12 + sudo: required + # Overrides `install` to set up custom asan flags + install: + - make sanitize + # Overrides `before_script` (tests are already run in `make sanitize`) + before_script: + - true + + ## ** Builds that do not get published ** + # g++ build (default builds all use clang++) + - os: linux + env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm" + node_js: 12 + addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - libstdc++-6-dev + - g++-6 + # Overrides `install` to avoid initializing clang toolchain + install: + - make ${BUILDTYPE} + # Overrides `script` to disable publishing + script: + - true + # Coverage build + - os: linux + env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" + node_js: 12 + # Overrides `script` to publish coverage data to codecov + script: + - export PATH=$(pwd)/mason_packages/.link/bin/:${PATH} + - which llvm-cov + - curl -S -f https://codecov.io/bash -o codecov + - chmod +x codecov + - ./codecov -x "llvm-cov gcov" -Z + # Clang format build + - os: linux + # can be generic since we don't need nodejs to run formatting + language: generic + env: CLANG_FORMAT + # Overrides `install` to avoid initializing clang toolchain + install: + # Run the clang-format script. Any code formatting changes + # will trigger the build to fail (idea here is to get us to pay attention + # and get in the habit of running these locally before committing) + - make format + # Overrides `before_script`, no need to run tests + before_script: + - true + # Overrides `script` to disable publishing + script: + - true + # Clang tidy build + - os: linux + env: CLANG_TIDY + node_js: 12 + # Overrides `install` to avoid initializing clang toolchain + install: + # First run the clang-tidy target + # Any code formatting fixes automatically applied by clang-tidy + # will trigger the build to fail (idea here is to get us to pay attention + # and get in the habit of running these locally before committing) + - make tidy + # Overrides `before_script`, no need to run tests + before_script: + - true # Overrides `script` to disable publishing script: + - true From eaa4fbfc8388e14766cd3f0c28413b384e34c040 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 13:12:33 +0100 Subject: [PATCH 42/55] tidy .travis.yml --- .travis.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index ccb99c1..a062a71 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,11 +31,7 @@ script: # run your tests and build binaries matrix: include: - ## ** Builds that are published ** - - # Node 10 - # linux cfi build node v10/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" @@ -139,8 +135,6 @@ matrix: # Overrides `script` to disable publishing script: - true - - # Node 12 # linux cfi build node v12/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" From 9eb4ab58a4b140d9be255b30cc419ec49ebe858b Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 13:58:23 +0100 Subject: [PATCH 43/55] Revert "tidy .travis.yml" This reverts commit eaa4fbfc8388e14766cd3f0c28413b384e34c040. --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index a062a71..ccb99c1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,7 +31,11 @@ script: # run your tests and build binaries matrix: include: + ## ** Builds that are published ** + + # Node 10 + # linux cfi build node v10/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" @@ -135,6 +139,8 @@ matrix: # Overrides `script` to disable publishing script: - true + + # Node 12 # linux cfi build node v12/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" From 0c6c1fb0b0fd8c239ad472e44955121d28286e2e Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 13:58:28 +0100 Subject: [PATCH 44/55] Revert "Only target node v10 and v12" This reverts commit bd6bdb6ca1d7d3b68efc367f22e7fd00dcb4a3c8. --- .travis.yml | 134 +++++++--------------------------------------------- 1 file changed, 17 insertions(+), 117 deletions(-) diff --git a/.travis.yml b/.travis.yml index ccb99c1..63533d6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,8 +34,6 @@ matrix: ## ** Builds that are published ** - # Node 10 - # linux cfi build node v10/release - os: linux env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" @@ -48,11 +46,24 @@ matrix: - os: linux env: BUILDTYPE=debug node_js: 10 + # linux publishable node v8/release + - os: linux + env: BUILDTYPE=release + node_js: 8 + # linux publishable node v8/debug + - os: linux + env: BUILDTYPE=debug + node_js: 8 # osx publishable node v10/release - os: osx osx_image: xcode9.3 env: BUILDTYPE=release node_js: 10 + # osx publishable node v8/release + - os: osx + osx_image: xcode9.3 + env: BUILDTYPE=release + node_js: 8 # linux sanitizer build node v10/debug - os: linux env: BUILDTYPE=debug TOOLSET=asan @@ -63,7 +74,6 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: - - true # osx sanitizer build node v10/debug - os: osx env: BUILDTYPE=debug TOOLSET=asan @@ -74,118 +84,13 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: - - true - - ## ** Builds that do not get published ** - # g++ build (default builds all use clang++) - - os: linux - env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm" - node_js: 10 - addons: - apt: - sources: - - ubuntu-toolchain-r-test - packages: - - libstdc++-6-dev - - g++-6 - # Overrides `install` to avoid initializing clang toolchain - install: - - make ${BUILDTYPE} - # Overrides `script` to disable publishing - script: - - true - # Coverage build - - os: linux - env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" - node_js: 10 - # Overrides `script` to publish coverage data to codecov - script: - - export PATH=$(pwd)/mason_packages/.link/bin/:${PATH} - - which llvm-cov - - curl -S -f https://codecov.io/bash -o codecov - - chmod +x codecov - - ./codecov -x "llvm-cov gcov" -Z - # Clang format build - - os: linux - # can be generic since we don't need nodejs to run formatting - language: generic - env: CLANG_FORMAT - # Overrides `install` to avoid initializing clang toolchain - install: - # Run the clang-format script. Any code formatting changes - # will trigger the build to fail (idea here is to get us to pay attention - # and get in the habit of running these locally before committing) - - make format - # Overrides `before_script`, no need to run tests - before_script: - - true - # Overrides `script` to disable publishing - script: - - true - # Clang tidy build - - os: linux - env: CLANG_TIDY - node_js: 10 - # Overrides `install` to avoid initializing clang toolchain - install: - # First run the clang-tidy target - # Any code formatting fixes automatically applied by clang-tidy - # will trigger the build to fail (idea here is to get us to pay attention - # and get in the habit of running these locally before committing) - - make tidy - # Overrides `before_script`, no need to run tests - before_script: - - true - # Overrides `script` to disable publishing - script: - - true - - # Node 12 - # linux cfi build node v12/release - - os: linux - env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-fsanitize=cfi -fvisibility=hidden" LDFLAGS="-fsanitize=cfi" - node_js: 12 - # linux publishable node v12/release - - os: linux - env: BUILDTYPE=release - node_js: 12 - # linux publishable node v12/debug - - os: linux - env: BUILDTYPE=debug - node_js: 12 - # osx publishable node v12/release - - os: osx - osx_image: xcode9.3 - env: BUILDTYPE=release - node_js: 12 - # linux sanitizer build node v12/debug - - os: linux - env: BUILDTYPE=debug TOOLSET=asan - node_js: 12 - sudo: required - # Overrides `install` to set up custom asan flags - install: - - make sanitize - # Overrides `before_script` (tests are already run in `make sanitize`) - before_script: - - true - # osx sanitizer build node v12/debug - - os: osx - env: BUILDTYPE=debug TOOLSET=asan - node_js: 12 - sudo: required - # Overrides `install` to set up custom asan flags - install: - - make sanitize - # Overrides `before_script` (tests are already run in `make sanitize`) - before_script: - - true ## ** Builds that do not get published ** + # g++ build (default builds all use clang++) - os: linux env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm" - node_js: 12 + node_js: 10 addons: apt: sources: @@ -198,11 +103,10 @@ matrix: - make ${BUILDTYPE} # Overrides `script` to disable publishing script: - - true # Coverage build - os: linux env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" - node_js: 12 + node_js: 10 # Overrides `script` to publish coverage data to codecov script: - export PATH=$(pwd)/mason_packages/.link/bin/:${PATH} @@ -223,14 +127,12 @@ matrix: - make format # Overrides `before_script`, no need to run tests before_script: - - true # Overrides `script` to disable publishing script: - - true # Clang tidy build - os: linux env: CLANG_TIDY - node_js: 12 + node_js: 10 # Overrides `install` to avoid initializing clang toolchain install: # First run the clang-tidy target @@ -240,7 +142,5 @@ matrix: - make tidy # Overrides `before_script`, no need to run tests before_script: - - true # Overrides `script` to disable publishing script: - - true From 3c8dfb6a9a03cc24b7aab6d99ae55ead0a9985c2 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 14:09:26 +0100 Subject: [PATCH 45/55] Fix (hopefully) .travis.yml syntax --- .travis.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 63533d6..980a9a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -74,6 +74,7 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: + - true # osx sanitizer build node v10/debug - os: osx env: BUILDTYPE=debug TOOLSET=asan @@ -84,7 +85,7 @@ matrix: - make sanitize # Overrides `before_script` (tests are already run in `make sanitize`) before_script: - + - true ## ** Builds that do not get published ** # g++ build (default builds all use clang++) @@ -103,6 +104,7 @@ matrix: - make ${BUILDTYPE} # Overrides `script` to disable publishing script: + - true # Coverage build - os: linux env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage" @@ -127,8 +129,10 @@ matrix: - make format # Overrides `before_script`, no need to run tests before_script: + - true # Overrides `script` to disable publishing script: + - true # Clang tidy build - os: linux env: CLANG_TIDY @@ -142,5 +146,7 @@ matrix: - make tidy # Overrides `before_script`, no need to run tests before_script: + - true # Overrides `script` to disable publishing script: + - true From 826dd48a35df0ca589110efb3c1bd5bd52af4c20 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 14:17:56 +0100 Subject: [PATCH 46/55] Update node versions --- .travis.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 980a9a0..2551b4e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,24 +46,24 @@ matrix: - os: linux env: BUILDTYPE=debug node_js: 10 - # linux publishable node v8/release + # linux publishable node v12/release - os: linux env: BUILDTYPE=release - node_js: 8 - # linux publishable node v8/debug + node_js: 12 + # linux publishable node v12/debug - os: linux env: BUILDTYPE=debug - node_js: 8 + node_js: 12 # osx publishable node v10/release - os: osx osx_image: xcode9.3 env: BUILDTYPE=release node_js: 10 - # osx publishable node v8/release + # osx publishable node v12/release - os: osx osx_image: xcode9.3 env: BUILDTYPE=release - node_js: 8 + node_js: 12 # linux sanitizer build node v10/debug - os: linux env: BUILDTYPE=debug TOOLSET=asan From c926694111950bc9bd2e711f784b9d26b8a18159 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 15:04:33 +0100 Subject: [PATCH 47/55] Add AsyncHelloWorker_v2 implementation which uses `GetResult` method and default `OnOk()` and `OnError`. --- src/object_async/hello_async.cpp | 60 +++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index f5d4d27..fc6c589 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -39,6 +39,7 @@ namespace object_async { +/* struct AsyncHelloWorker : Napi::AsyncWorker { using Base = Napi::AsyncWorker; @@ -102,6 +103,63 @@ struct AsyncHelloWorker : Napi::AsyncWorker bool const buffer_; std::string const name_; }; +*/ + +// This V2 worker is overriding `GetResult` to return the arguments +// passed to the Callback invoked by the default OnOK() implementation. +// Above is alternative implementation with OnOK() method calling +// Callback with appropriate args. Both implementations use default OnError(). +struct AsyncHelloWorker_v2 : Napi::AsyncWorker +{ + using Base = Napi::AsyncWorker; + // ctor + AsyncHelloWorker_v2(bool louder, + bool buffer, + std::string name, + Napi::Function const& cb) + : Base(cb), + louder_(louder), + buffer_(buffer), + name_(std::move(name)) {} + + // The Execute() function is getting called when the worker starts to run. + // - You only have access to member variables stored in this worker. + // - You do not have access to Javascript v8 objects here. + void Execute() override + { + try + { + result_ = detail::do_expensive_work(name_, louder_); + } + catch (std::exception const& e) + { + SetError(e.what()); + } + } + + std::vector GetResult(Napi::Env env) override + { + if (buffer_) + { + auto buffer = Napi::Buffer::New(env, + result_->data(), + result_->size(), + [](Napi::Env, char*, gsl::owner*> v) { + delete v; + }, + result_.release()); + return {env.Null(), buffer}; + } + return {env.Null(), Napi::String::New(env, result_->data(), result_->size())}; + + } + + std::unique_ptr> result_ = nullptr; + bool const louder_; + bool const buffer_; + std::string const name_; +}; + Napi::FunctionReference HelloObjectAsync::constructor; // NOLINT @@ -168,7 +226,7 @@ Napi::Value HelloObjectAsync::helloAsync(Napi::CallbackInfo const& info) buffer = buffer_val.As().Value(); } - auto* worker = new AsyncHelloWorker{louder, buffer, name_, callback}; // NOLINT + auto* worker = new AsyncHelloWorker_v2{louder, buffer, name_, callback}; // NOLINT worker->Queue(); return info.Env().Undefined(); // NOLINT } From d2cb8186052a0485aec897fff90be2d132a3d8f4 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 15:06:50 +0100 Subject: [PATCH 48/55] add node v13 targets --- .travis.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.travis.yml b/.travis.yml index 2551b4e..91a4b28 100644 --- a/.travis.yml +++ b/.travis.yml @@ -54,6 +54,14 @@ matrix: - os: linux env: BUILDTYPE=debug node_js: 12 + # linux publishable node v13/release + - os: linux + env: BUILDTYPE=release + node_js: 13 + # linux publishable node v13/debug + - os: linux + env: BUILDTYPE=debug + node_js: 13 # osx publishable node v10/release - os: osx osx_image: xcode9.3 From 885b623a85f8bb2f54f9f6b9a9288d944fb8b215 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Wed, 1 Apr 2020 15:09:09 +0100 Subject: [PATCH 49/55] clang-format --- src/object_async/hello_async.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index fc6c589..c4577ba 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -114,9 +114,9 @@ struct AsyncHelloWorker_v2 : Napi::AsyncWorker using Base = Napi::AsyncWorker; // ctor AsyncHelloWorker_v2(bool louder, - bool buffer, - std::string name, - Napi::Function const& cb) + bool buffer, + std::string name, + Napi::Function const& cb) : Base(cb), louder_(louder), buffer_(buffer), @@ -151,7 +151,6 @@ struct AsyncHelloWorker_v2 : Napi::AsyncWorker return {env.Null(), buffer}; } return {env.Null(), Napi::String::New(env, result_->data(), result_->size())}; - } std::unique_ptr> result_ = nullptr; @@ -160,7 +159,6 @@ struct AsyncHelloWorker_v2 : Napi::AsyncWorker std::string const name_; }; - Napi::FunctionReference HelloObjectAsync::constructor; // NOLINT HelloObjectAsync::HelloObjectAsync(Napi::CallbackInfo const& info) From 11e9edb824ba9ce7557693e24c9f973f21c3d9fb Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 2 Apr 2020 14:55:25 +0100 Subject: [PATCH 50/55] Don't rely on order of args evaluation which is implemetation defined (e.g clang++ differs from GCC) + check `result_` before deref etc. C++ standard 5.2.2.8 : "The order of evaluation of arguments is unspecified. All side effects of argument expression evaluations take effect before the function is entered. The order of evaluation of the postfix expression and the argument expression list is unspecified." http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1905.pdf --- src/object_async/hello_async.cpp | 32 +++++++++++++++++----------- src/standalone_async/hello_async.cpp | 8 ++++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/object_async/hello_async.cpp b/src/object_async/hello_async.cpp index c4577ba..a36ef63 100644 --- a/src/object_async/hello_async.cpp +++ b/src/object_async/hello_async.cpp @@ -82,9 +82,11 @@ struct AsyncHelloWorker : Napi::AsyncWorker { if (buffer_) { + char * data = result_->data(); + std::size_t size = result_->size(); auto buffer = Napi::Buffer::New(Env(), - result_->data(), - result_->size(), + data, + size, [](Napi::Env, char*, gsl::owner*> v) { delete v; }, @@ -139,18 +141,24 @@ struct AsyncHelloWorker_v2 : Napi::AsyncWorker std::vector GetResult(Napi::Env env) override { - if (buffer_) + if (result_) { - auto buffer = Napi::Buffer::New(env, - result_->data(), - result_->size(), - [](Napi::Env, char*, gsl::owner*> v) { - delete v; - }, - result_.release()); - return {env.Null(), buffer}; + if (buffer_) + { + char* data = result_->data(); + std::size_t size = result_->size(); + auto buffer = Napi::Buffer::New(env, + data, + size, + [](Napi::Env, char*, gsl::owner*> v) { + delete v; + }, + result_.release()); + return {env.Null(), buffer}; + } + return {env.Null(), Napi::String::New(env, result_->data(), result_->size())}; } - return {env.Null(), Napi::String::New(env, result_->data(), result_->size())}; + return Base::GetResult(env); // returns an empty vector (default) } std::unique_ptr> result_ = nullptr; diff --git a/src/standalone_async/hello_async.cpp b/src/standalone_async/hello_async.cpp index 0514d17..e2726a6 100644 --- a/src/standalone_async/hello_async.cpp +++ b/src/standalone_async/hello_async.cpp @@ -69,13 +69,15 @@ struct AsyncHelloWorker : Napi::AsyncWorker void OnOK() final { Napi::HandleScope scope(Env()); - if (!Callback().IsEmpty()) + if (!Callback().IsEmpty() && result_) { if (buffer_) { + char* data = result_->data(); + std::size_t size = result_->size(); auto buffer = Napi::Buffer::New(Env(), - result_->data(), - result_->size(), + data, + size, [](Napi::Env, char*, gsl::owner*> v) { delete v; }, From 65da09ad8ca8fbcf83aef97548bc8691e74aaef6 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 2 Apr 2020 15:57:52 +0100 Subject: [PATCH 51/55] Makefile - better target name --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index ba7bd3b..7876bbb 100644 --- a/Makefile +++ b/Makefile @@ -24,10 +24,10 @@ export WERROR ?= true # just typing `make` will call `make release` default: release -node_modules/nan: +node_modules/install: npm install --ignore-scripts -mason_packages/headers: node_modules/nan +mason_packages/headers: node_modules/install node_modules/.bin/mason-js install mason_packages/.link/include: mason_packages/headers From 7c717d9248137d727722ccf9c5b9f6ad43aee873 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Thu, 2 Apr 2020 16:00:33 +0100 Subject: [PATCH 52/55] Update link [skip ci] --- docs/extended-tour.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/extended-tour.md b/docs/extended-tour.md index 4de4a27..c113b38 100644 --- a/docs/extended-tour.md +++ b/docs/extended-tour.md @@ -21,7 +21,7 @@ This skeleton includes a few examples of how you might design your application, ### When would you use a standalone function? -A standalone function is a function that exists at the top level of the module scope rather than as a member of an object that is instantiated. So if your module is `wonderful`, a standalone function would be called like `wonderful.callme()`. +A standalone function is a function that exists at the top level of the module scope rather than as a member of an object that is instantiated. So if your module is `wonderful`, a standalone function would be called like `wonderful.callme()`. A standalone function makes sense when the only data needed by the function can be easily passed as arguments. When it is not easy or clean to pass data as arguments then you should consider encapsulation, for example, exposing a function as a member of an object. One of the benefits of creating a standalone function is that it can help [separate concerns](https://en.wikipedia.org/wiki/Separation_of_concerns), which is mainly a stylistic design decision. @@ -32,12 +32,12 @@ Example: ### When would you use an object/class? Create an object/class when you need to do some kind of data preprocessing before going into the thread pool. It's best to write your code so that the preprocessing happens _once_ as a separate operation, then continues through to the thread pool after the preprocessing is finished and the object is ready. Examples: - [node-mapnik](https://github.com/mapnik/node-mapnik/blob/fe80ce5d79c0e90cfbb5a2b992bf0ae2b8f88198/src/mapnik_map.hpp#L20): we create a Map object once, then use the object multiple times for rendering each vector tile. - - [node-addon-examples](https://github.com/nodejs/node-addon-examples/tree/master/6_object_wrap/nan) for another example of what an object-focused Addon looks like. + - [node-addon-examples](https://github.com/nodejs/node-addon-examples/tree/master/6_object_wrap/node-addon-api) for another example of what an object-focused Addon looks like. Objects/Classes are useful when you start doing more complex operations and need to consider performance more heavily, when performance constraints start to matter. Use classes to compute the value of something once, rather than every time you call a function. One step further is using an asynchronous object or class, which enables you to pass data into the threadpool. The aim is to process data in the threadpool in the most efficient way possible. [Efficiency](https://github.com/mapbox/cpp/blob/master/glossary.md#efficiency) and high performance are the main goals of the `HelloObjectAsync` example in this skel. The `HelloObjectAsync` example demonstrates high load, when _many_ objects in _many_ threads are being allocated. This scenario is where reducing unnecessary allocations really pays off, and is also why this example uses "move semantics" for even better performance. -- **Move semantics**: move semantics avoid data being copied (allocating memory), to limit unnecessary memory allocation, and are most useful when working with big data. +- **Move semantics**: move semantics avoid data being copied (allocating memory), to limit unnecessary memory allocation, and are most useful when working with big data. Other thoughts related to move semantics: - Always best to use move semantics instead of passing by reference, espeically with objects like [`std::string`](http://shaharmike.com/cpp/std-string/), which can be expensive. @@ -175,7 +175,7 @@ This binary file `../lib/binding/module.node` is what `require()` points to with # Clang Tools -This skeleton uses two clang/llvm tools for automated formatting and static fixes. +This skeleton uses two clang/llvm tools for automated formatting and static fixes. Each of these tools run within their own [Travis jobs](https://github.com/mapbox/node-cpp-skel/blob/master/.travis.yml). You can disable these Travis jobs if you'd like, by commenting them out in `.travis.yml`. This may be necessary if you're porting this skel into an already-existing project that triggers tons of clang-tidy errors, and you'd like to work through them gradually while still actively iterating on other parts of your code. From 5cb474786dd32352c0a4b4b6e9ba5c6fe58b94ef Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 2 Apr 2020 09:50:14 -0700 Subject: [PATCH 53/55] attempt to fix coverage problem --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 91a4b28..d26a75b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -123,6 +123,8 @@ matrix: - which llvm-cov - curl -S -f https://codecov.io/bash -o codecov - chmod +x codecov + # Workaround until we can avoid problem after https://github.com/travis-ci/travis-build/pull/1263 lands + - PATH=$(echo "$PATH" | sed 's/.\/node_modules\/.bin://') - ./codecov -x "llvm-cov gcov" -Z # Clang format build - os: linux From e88d774f62788c6210b7beb3b9502aec7cc5b507 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 2 Apr 2020 09:58:44 -0700 Subject: [PATCH 54/55] suppress OS X specific leak --- scripts/sanitize.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/sanitize.sh b/scripts/sanitize.sh index 4168cca..2061287 100755 --- a/scripts/sanitize.sh +++ b/scripts/sanitize.sh @@ -25,6 +25,8 @@ echo "leak:__strdup" > ${SUPPRESSION_FILE} echo "leak:v8::internal" >> ${SUPPRESSION_FILE} echo "leak:node::CreateEnvironment" >> ${SUPPRESSION_FILE} echo "leak:node::Init" >> ${SUPPRESSION_FILE} +# Suppress leak related to https://github.com/libuv/libuv/pull/2480 +echo "leak:uv__set_process_title_platform_init" >> ${SUPPRESSION_FILE} export ASAN_SYMBOLIZER_PATH=$(pwd)/mason_packages/.link/bin/llvm-symbolizer export MSAN_SYMBOLIZER_PATH=$(pwd)/mason_packages/.link/bin/llvm-symbolizer export UBSAN_OPTIONS=print_stacktrace=1 From bb963f9af8a9ace62b82d89e2b6daf4aa59f0beb Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 2 Apr 2020 10:12:48 -0700 Subject: [PATCH 55/55] try removing old leak suppressions --- scripts/sanitize.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/sanitize.sh b/scripts/sanitize.sh index 2061287..c9b8543 100755 --- a/scripts/sanitize.sh +++ b/scripts/sanitize.sh @@ -21,10 +21,6 @@ fi export MASON_LLVM_RT_PRELOAD=$(pwd)/$(ls mason_packages/.link/lib/clang/*/lib/*/libclang_rt.asan*${SHARED_LIB_EXT}) SUPPRESSION_FILE="/tmp/leak_suppressions.txt" -echo "leak:__strdup" > ${SUPPRESSION_FILE} -echo "leak:v8::internal" >> ${SUPPRESSION_FILE} -echo "leak:node::CreateEnvironment" >> ${SUPPRESSION_FILE} -echo "leak:node::Init" >> ${SUPPRESSION_FILE} # Suppress leak related to https://github.com/libuv/libuv/pull/2480 echo "leak:uv__set_process_title_platform_init" >> ${SUPPRESSION_FILE} export ASAN_SYMBOLIZER_PATH=$(pwd)/mason_packages/.link/bin/llvm-symbolizer