vtshaver
vtshaver copied to clipboard
Fix last clang-tidy warnings
Per @springmeyer and @joto 's thoughts:
We have a few clang-tidy warnings needing fixed.
They should be failing the build, but are not due to https://github.com/mapbox/node-cpp-skel/issues/105.
These are what show up for me with make tidy locally on OS X:
../src/filters.cpp:50:13: error: initializing non-owner 'Filters *const' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
auto* const self = new Filters();
^
../src/shave.cpp:152:9: error: initializing non-owner 'AsyncBaton *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
auto* baton = new AsyncBaton();
^
../src/shave.cpp:342:60: error: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory,-warnings-as-errors]
delete reinterpret_cast<std::string*>(hint);
^
../src/shave.cpp:341:66: note: variable declared here
[](char*, void* hint) {
^
../src/shave.cpp:353:5: error: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory,-warnings-as-errors]
delete baton;
^
../src/shave.cpp:329:5: note: variable declared here
auto* baton = static_cast<AsyncBaton*>(req->data);
^
Now, I noticed that @joto just disabled this clang-tidy warning (`cppcoreguidelines-owning-memory) today in vtzero which indicates to be we may want to consider the same thing: https://github.com/mapbox/vtzero/commit/6822c59e17a6d51196d78fce6d2a5d383522c6e9
I think the cppcoreguidelines-owning-memory only makes sense if you buy into the whole gsl library. Each project has to decide whether it makes sense for them, but I think the gsl library makes more sense for higher level applications while lower-level libraries a) need more bit-twiddling and pointer-twiddling etc. anyway so they need to disable lots of these warnings and b) any extra dependency on low-level libraries makes they use that much harder.
Several of these warnings @alliecrevier fixed in node-cpp-skel as well: https://github.com/mapbox/node-cpp-skel/pull/129. So we can learn from those commits to address these.