Bug 220002 - Define _LIBCPP_ENABLE_NODISCARD when building WebKit
Summary: Define _LIBCPP_ENABLE_NODISCARD when building WebKit
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-17 19:22 PST by David Kilzer (:ddkilzer)
Modified: 2020-12-18 10:57 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 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.
Comment 1 Radar WebKit Bug Importer 2020-12-17 21:10:22 PST
<rdar://problem/72453662>
Comment 2 Joseph Pecoraro 2020-12-18 10:41:33 PST
Nice!
Comment 3 Joseph Pecoraro 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