Proposing google-default-arguments clang-tidy check

[Julie Jeongeun Kim]

Hello folks,

I'd like to propose that we enable the google-default-arguments clang-tidy lint. The evaluation doc I have is here.
As I described in the document, nothing related to google-default-arguments was observed across Chromium, but I think it would be good to have to prevent additional code in the future.

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

Feedback is appreciated!

Thanks,
Julie

[Avi Drissman]

This is already prohibited by the style guide, so having tooling to enforce it IMO is a good idea.

--
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/276eaa14-a962-497e-a425-747297022574n%40chromium.org.

[Peter Kasting]

Did you check if there is any existing clang warning that fires if we try to add a default arg on a virtual function?

If there is, then this is unnecessary. If not, then as Avi mentioned, catching the style violation with a tool would be good.

PK

[Lei Zhang]

I don't think there's an existing warning for this, as developers have
checked in code with virtual functions and default arguments. e.g. [1]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/4643671/5/chrome/browser/ui/browser_window.h#595

> --
> 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/CAAHOzFBdBUwK5Wr2-pZvJCmEVStLyyi9_TcVe8vQbDdGLCb4UQ%40mail.gmail.com.

[K. Moon]

I've definitely been guilty of this (and then having to fix it up later). I think it was harmless in practice because the default was consistent in the particular usage, but it was easy to make bad things happen.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACSHbcRhuE%2B-EVvWfa0gAg7QKoGwqhF%2Bgx-7%3DJzoEX8wGigLbg%40mail.gmail.com.

[퓨루나]

> Did you check if there is any existing clang warning that fires if we try to add a default arg on a virtual function?

I've checked it by adding a default arg in an existing method, but I didn't see any warning.

2023년 12월 13일 수요일 오전 2시 39분 13초 UTC+9에 K. Moon님이 작성:

[Julie Jeongeun Kim]

Thanks!

Here are the issue[1] and CLs[2] for enabling this check.

[1] https://crbug.com/1513346

[2] https://chromium-review.googlesource.com/c/v8/v8/+/5138842

[3] https://chromium-review.googlesource.com/c/chromium/src/+/5138982