Proposing bugprone-bool-pointer-implicit-conversion clang-tidy check

827 views
Skip to first unread message

Miyoung Shin

unread,
Dec 11, 2023, 11:16:02 AM12/11/23
to c...@chromium.org

Hello folks,

I'd like to propose that we enable the bugprone-bool-pointer-implicit-conversion clang-tidy lint. The evaluation doc I have is here.

I think the key points for discussion are:
- Do we want to enable this check?

Feedback is appreciated!

Thanks,
Miyoung Shin

Peter Kasting

unread,
Dec 12, 2023, 11:52:10 AM12/12/23
to Miyoung Shin, c...@chromium.org
Despite this only firing on one non-bug, I think this is worth enabling, as it's a rare-but-poor pattern.

The flagged code is tricky to understand because it's basically trying to combine two distinct tree-walking implementations into a single one. IMO, this code would be better if we split the two cases apart, wrote each clearly and optimally, and then looked to see if there was any opportunity to coalesce.

In general, bool* is a sketchy type, and I think having more checks around its usage is warranted.

PK

Avi Drissman

unread,
Dec 12, 2023, 11:59:27 AM12/12/23
to Peter Kasting, Miyoung Shin, c...@chromium.org
"bool*" is infamous inside of Google for being the cause of a very nasty user-visible incident that was very close to having permanent data loss. I am strongly in favor of this check.

Avi

--
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/CAAHOzFCmGxW_hR5%2BKMsN%2BNw%2B5bh07j%2BjCkD8AnK_Tao9h%3Dvtxg%40mail.gmail.com.

Miyoung Shin

unread,
Jan 4, 2024, 11:06:59 AM1/4/24
to cxx, a...@chromium.org, Miyoung Shin, c...@chromium.org, pkas...@google.com
Thanks all!
I filed https://crbug.com/1515513, fixed the existing warning[1] and have an uploaded CL[2] to enabling it.

2023년 12월 13일 수요일 오전 1시 59분 27초 UTC+9에 a...@chromium.org님이 작성:
Reply all
Reply to author
Forward
0 new messages