Skip to content

Commit

Permalink
use nolint as workaround to misc-unused-parameters bug
Browse files Browse the repository at this point in the history
  • Loading branch information
Allie Crevier committed Jun 6, 2018
1 parent d058f63 commit 892723b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
10 changes: 5 additions & 5 deletions src/object_async/hello_async.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,17 @@ NAN_METHOD(HelloObjectAsync::New) {
*/
std::string name(*utf8_value, static_cast<std::size_t>(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)
* 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
* 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
**/
Expand Down Expand Up @@ -155,7 +155,7 @@ struct AsyncHelloWorker : Nan::AsyncWorker // NOLINT to disable cppcoreguideline

AsyncHelloWorker(bool louder, const std::string* name,
Nan::Callback* cb)
: Base(cb), , louder_{louder}, name_{name} {}
: Base(cb), result_{}, louder_{louder}, name_{name} {} // NOLINT TODO: disable misc-unused-parameters check

This comment has been minimized.

Copy link
@springmeyer

springmeyer Jun 6, 2018

Contributor

Take a look at mapbox/cpp#54. In short @AllieOop what do you think about removing the result from the initializer list but doing brace initialization in the member (probably not using my C++ terms perfectly here...) like 7a97785 for a fix instead of NOLINT?

// The Execute() function is getting called when the worker starts to run.
// - You only have access to member variables stored in this worker.
Expand Down
2 changes: 1 addition & 1 deletion src/standalone_async/hello_async.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ struct AsyncHelloWorker : Nan::AsyncWorker {
using Base = Nan::AsyncWorker;

AsyncHelloWorker(bool louder, Nan::Callback* cb)
: Base(cb), , louder_{louder} {}
: Base(cb), result_{}, louder_{louder} {} // NOLINT TODO: disable misc-unused-parameters check

// The Execute() function is getting called when the worker starts to run.
// - You only have access to member variables stored in this worker.
Expand Down

0 comments on commit 892723b

Please sign in to comment.