NEW 220002
Define _LIBCPP_ENABLE_NODISCARD when building WebKit
https://bugs.webkit.org/show_bug.cgi?id=220002
Summary Define _LIBCPP_ENABLE_NODISCARD when building WebKit
David Kilzer (:ddkilzer)
Reported 2020-12-17 19:22:02 PST
Define _LIBCPP_ENABLE_NODISCARD when building WebKit. This will catch errors where a developer means to declare a variable (like a resource acquisition is initialization (RAII) object), but forgets to give it a variable name: SpmeRAIIObject { }; This instantiates the object then destroys it in a single statement because the object's scope is the statement itself. The developer probably meant to give a variable like this instead: SpmeRAIIObject object { }; Note that I tried compiling a Release build of WebKit for macOS with this enabled, but I didn't find any issues.
Attachments
Radar WebKit Bug Importer
Comment 1 2020-12-17 21:10:22 PST
Joseph Pecoraro
Comment 2 2020-12-18 10:41:33 PST
Nice!
Joseph Pecoraro
Comment 3 2020-12-18 10:54:42 PST
(In reply to David Kilzer (:ddkilzer) from comment #0) > Define _LIBCPP_ENABLE_NODISCARD when building WebKit. > > This will catch errors where a developer means to declare a variable (like a > resource acquisition is initialization (RAII) object), but forgets to give > it a variable name: > > SpmeRAIIObject { }; To be clear the `_LIBCPP_ENABLE_NODISCARD` only adds the `[[nodiscard]]` annotation to more of the `std` Standard Library Functions than already have it since C++17. --- If WebKit has its own classes where this is important (Lock Guards / Mutex Holders, Allocation / Copy / Ownership passing methods) then they should be annotated with `[[nodiscard]]` in order to get such warnings. Currently I see no `[[nodiscard]]` use in WebKit, but there is `WARN_UNUSED_RETURN` which is probably pretty similar. Things like: * RAII constructors - WTF::Locker for example * Ownership Passing methods that may leak if the result is not used - WTF::MallocPtr::leakPtr for example * Allocation methods that may leak if the result is not used - WTF::fastMalloc for example
Note You need to log in before you can comment on or make changes to this bug.