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

[Jan Voung via cfe-commits]

jvoung wrote:

> Sorry for the delay.
> 
> I think, that we can probably all agree that the best solution would be to have the model support the macros, e.g., by recognizing the patterns that are behind these macros. However, that is currently not something that people can find time to do (as was communicated by @ymand w.r.t. current priorities), and not something that seems to have caused significant enough irritation for someone to step in themselves.
> 
> As discussed, disabling the check inside the test folder only works if the project structure supports it (which is IMO the majority, but that is just a feeling), but that is also something users have to know they can do. We should open an issue with the help wanted label, and mention explicitly that this is not simple in the description. That way, someone may be inclined to start working on this.
> 

Thanks, that is a good summary =)

(and sorry for not replying earlier about the test folder alternative, but as @ymand mentioned, our codebase unfortunately doesn't use test folders)

> I think the GTest matchers don't work here, because you'd want to mark the optional as having a value or not having a value, and you would need access to the environment at that time, which is not available at this stage. But the GTest macros could be used for a crude implementation inside the model, right?

Yes, the analysis would want to mark the optional as having a value or not in the environment. So modeling would be a combination of the GTest matchers to dispatch to some transfer function, and then the transfer functions will modify the environment appropriately.  Similarly for catch2 matchers + transfer functions, etc. 

There was also the suggestion from Gabor of adding `analyzer_noreturn` attributes. But it would also take some time to modify the appropriate libraries and wait for roll out, etc. Or perhaps similarly, if the libraries could be modified to provide super-simple versions of the macro behind ifdef __clang_analyzer__ (e.g., for googletest, skip the `Converter` ctor + operator bool function crossings), but again take time to see if the libraries are willing, modify, wait for rollout, etc.

> I'm not really that opposed to this solution, but the other solutions sound better to me, though with their own caveats (time, work, GTest macros are only for GTest). I'm fine with this, if all other options are not feasible to implement in the near to mid term, but a second non-blocking review from another clang-tidy dev would be best.
> 
> Added two more, but there are some other review comments still open, probably because of the discussion.

Ah sorry, yes, I hadn't addressed the earlier review comments since it seemed like bigger picture discussion was still open. Update the patch.


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