[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

Wed Jun 21 07:25:06 PDT 2023

steakhal resigned from this revision.
steakhal added a comment.

I resign as a reviewer as I'm not deeply connected to this checker, thus I won't block it or accept it.
However, my opinion is that a checker should be "released" if they have clear diagnostics (which includes that it doesn't flood the user with unimportant diagnostics either).
Consequently, if there are features missing to accomplish that, then that thing is a blocker.

TBH I never understood why interestingness is not transitive over the `SymExpr` dependencies (`symbols_begin/end`).
This was not the only case when it hindered us. Just think of how taint propagates <https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/Taint.cpp#L261-L263>.

In D152436#4437692 <https://reviews.llvm.org/D152436#4437692>, @donat.nagy wrote:

> Personally I think it's completely acceptable if the analyzer sometimes emits bug reports that are true positives but lack a message [...]

I must admit that I'm in the other camp.

> If these issues produce //lots// of issues that are //very// confusing, then we should put the affected functions behind off-by-default flags, finish this review process, and revisit them later when the interestingness system is improved; but based on the available information I don't think that's necessary.

I can agree with this pragmatic approach.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152436/new/

https://reviews.llvm.org/D152436