| Summary: | check-webkit-style should warn about 'decode' functions with missing WARN_UNUSED_RETURN attribute | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, darin, ews-watchlist, glenn, jbedard, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=210459 | ||||||||
| Attachments: |
|
||||||||
|
Description
David Kilzer (:ddkilzer)
2020-04-10 08:45:54 PDT
Created attachment 396088 [details]
Patch v1
Comment on attachment 396088 [details]
Patch v1
For functions returning bool I think this is a good rule. For ones returning Optional I think this is not helpful, but I suppose no big deal to comply anyway.
Comment on attachment 396088 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=396088&action=review > Tools/Scripts/webkitpy/style/checkers/cpp.py:1818 > + if function_state.has_return_type('(auto|bool|Optional<.+>)'): I would actually expect this to be an inverse check. If a function does not have a void return type, then we should trigger this logic. Any reason we picked this set of types? Comment on attachment 396088 [details]
Patch v1
Darin r+ed it, I accidentally removed the r+ when adding a comment
If we are going to consider return types, the real issue is about bool return types. I don’t see a real unused return value issue outside of the bool return type case. Comment on attachment 396088 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=396088&action=review > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:3039 > + ' static WARN_UNUSED_RETURN Optional<std::tuple<>> decode(Decoder&)\n' This shouldn't be added. Comment on attachment 396088 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=396088&action=review >> Tools/Scripts/webkitpy/style/checkers/cpp.py:1818 >> + if function_state.has_return_type('(auto|bool|Optional<.+>)'): > > I would actually expect this to be an inverse check. If a function does not have a void return type, then we should trigger this logic. Any reason we picked this set of types? This set of types are specific to the decode() methods we care about. I'm going to remove Optional<.+> from the list per feedback from Alex and Darin. >> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:3039 >> + ' static WARN_UNUSED_RETURN Optional<std::tuple<>> decode(Decoder&)\n' > > This shouldn't be added. I will remove "Optional<.+>" from the list of types based on feedback from you and Darin, and remove this test (since it's a dupe of the one above) before landing. Created attachment 396158 [details]
Patch for landing
Committed r259925: <https://trac.webkit.org/changeset/259925> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396158 [details]. |