Adding more bugprone clang-tidy checks to Chromium [2/N]

[Jan Wilken Dörrie]

Hi all,

Following gbiv@'s example here I would like to propose to add the following bugprone clang-tidy checks:

bugprone-string-constructor: This check flags suspicious string constructor usages. It issues warnings if a string is created from a (data, size) pair and the size doesn't match the length of the string literal, likely misuses of the (count, character) constructor, or if a string is constructed from a nullptr, resulting in UB. This check currently fires 18 times in the codebase, with effectively no false positives. See this doc for details.

bugprone-dangling-handle: This check fires if a temporary is passed to a handle class like std::string_view, resulting in a dangling reference. This check is not perfect, but does not create false positives. While tools like ASAN are likely able to catch these errors as well, being able to do this statically seems like a net win. The list of handle classes is customizable, and I propose to include base::BasicStringPiece and base::span. This currently fires twice in Chromium.

bugprone-inaccurate-erase: This check fires if the erase-remove idiom is applied incorrectly. In these cases the iterator returned by std::remove is passed to the single iterator erase overload, instead of using the range overload and passing the end() iterator as well. This is subtle and can hide bugs, since it works correctly in case a single element should be erased. Flagging this should be another win without generating false positives. This currently fires three times in Chromium.
Note: Ideally we would point developers to base::Erase(If) instead, which is concise and correct. However, this would require maintaining our custom version of this check, and thus I'd like to leave it for future work :)

Please let me know what you think about these checks. If I don't receive objections I will likely land a CL enabling the checks next week.

Best,

Jan

[dan...@chromium.org]

Thanks Jan. LGTM

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALB5StYrUSbMwtdWeyi-7tbfyA4wfZP4Zar56vHEyxaDJHhQ7w%40mail.gmail.com.

[Gabriel Charette]

+1, thanks Jan

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaTU%3DTAvUJURvA4Zp_4vaZG-zFN_HJg%3D6XRi0-_1iuojgA%40mail.gmail.com.

[Jan Wilken Dörrie]

Thanks, both! This is now live: https://crrev.com/c/2584937

[George Burgess]

Awesome. Thanks for working on this! :)