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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
