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