Enabling bugprone-argument-comment check in Tricium

[Tal Pressman]

Hi all,

A recent discussion on a CL got me to try to enable clang-tidy's bugprone-argument-comment check in Tricium.

The check involves making sure that argument name comments in function call sites (i.e., f(/*foo=*/true, /*bar=*/5)) match the parameter name in the function declaration. Following the guidelines for enabling new checks, this is the evaluation document, and these are the full results.

TL;DR of the document:

• The analyzer has a hard time with gMock - many of the macros use generic parameter names (p0, gmock_p1, etc.) instead of the names of the parameters they are mocking.
• Many of the warnings appear to be as a result of renaming the parameter (or refactoring APIs), that did not update the comments. Unfortunately, cases like this will likely not be caught by Tricium, as it only analyzes modified code, without its usages. (See, for example, https://crrev.com/c/2557940, that correctly warns about the comments in agent_scheduling_group.cc:61-62 but does not warn about lines 74-75 where the declaration changed in another file.)

So what do you think? Yay or nay?

Thanks,

Tal

[Kouhei Ueno]

+1 to enable.

--
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/4ab04c64-72dd-4c61-bef9-60ca0247b5b6n%40chromium.org.

--

kouhei

[Jan Wilken Dörrie]

+1 from me as well. The gMock false positives are unfortunate but seem to be rare enough. Being able to catch errors like confusing "width" and "height" seems to be worth it in my opinion.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAPVAxLUzj00ctO54vjFe7Bnc31cQsDp0aupDNf6c3eUHOvV3KA%40mail.gmail.com.

[Kyle Charbonneau]

+1 as well

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALB5StZhMsB7J7-YRMaOEjmpzmLCy9irbieOE69-OvXtiTQ5Eg%40mail.gmail.com.

[Tal Pressman]

https://crrev.com/c/2557940 was submitted, enabling this check.

Tal