[clang-tools-extra] [clang-tidy] Filter out googletest TUs in bugprone-unchecked-optional-access (PR #115051)

Jan Voung via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 7 05:52:32 PST 2024


jvoung wrote:

> > > we're not (fully) understanding the content
> > 
> > 
> > My thinking was that we don't even need to understand the content, we simply exclude code that is contained within any of the problematic public macros. This sounds like it should be possible to do? Unfortunately I don't know the details on how this could be implemented, hopefully other reviewers know better?
> > Otherwise ChatGPT seems to give useful ideas on how to skip a matched result contained within an `ASSERT` macro (obviously untested):
> > ```
> >   if (Lexer::getImmediateMacroName(Loc, SM, Result.Context->getLangOpts()) == "ASSERT") {
> >     // The call is within ASSERT, no diagnostic needed.
> >     return;
> >   }
> > ```
> 
> That doesn't handle some cases like:
> 
> ```
> auto opt = DoSomeSetup(...)
> ASSERT_TRUE(opt.has_value())
> T x = DoMoreSetup(*opt)  // warn right here, since we didn't interpret the above ASSERT_TRUE (or other ways to check)
> 
> EXPECT_EQ(FunctionToTest(x), ...);
> ```
> 
> Sometimes the `*opt` may be within a macro, but not always.

- In non-test code, it is even more likely that the `*opt` is not wrapped in a macro, while the `ASSERT(opt.has_value())` is.
- If in non-test scenarios, the `ASSERT` macro implementation is actually simple, and the analysis already understood `ASSERT(opt.has_value())` makes a following `*opt` safe, then ignoring the `ASSERT` is actually worse and causes false positives.
- We wouldn't want to accidentally ignore the wrong macros (especially in non-test contexts)
- Coming up with a list of exactly the right macros to ignore for gtest, would be a bigger list of public macro names than the current change.
- And it still doesn't solve the "sometimes the `*opt` may be within a macro, but not always."

https://github.com/llvm/llvm-project/pull/115051


More information about the cfe-commits mailing list