search

mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

LLVM 7 #154

Closed springmeyer closed 6 years ago

springmeyer commented 6 years ago

Upgrades to LLVM 7

Refs https://github.com/mapbox/mason/pull/665

springmeyer commented 6 years ago

@flippmoke requested your review - would like your quick gut check on whether there is a better way (than NOLINT) to avoid the bugprone-unused-return-value warnings, which are new in LLVM 7.

flippmoke commented 6 years ago

@springmeyer did a quick review of this all again and I do not think there is a better way. Its just one of those weird gotcha situations where this is actually useful. Not releasing until the isEmpty() check makes sure that the constructor for the Buffer actually took ownership of the data from the pointer.

I can think of some other ways to possible do this to make sure that the pointer is always deleted, but then we would have to insert a try/catch here and I am not sure that is ideal either. Overall, I would call this a very valid use of NOLINT.

springmeyer commented 6 years ago

Thanks for review @flippmoke