[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Do I understand correctly that for the time being, the strategy is to assume apiModeling to be enabled by default and not enforce it explicitly?

I suppose I have the ol' reliable question: did you evaluate this on any project yet?

In D135247#3839543 <https://reviews.llvm.org/D135247#3839543>, @martong wrote:

>> The two checkers work together and 'apiModeling.StdCLibraryFunctions'
>> and its 'ModelPOSIX=true' option should be now a dependency of
>> checker 'alpha.unix.Stream'.
>
> Is there a way to stop the analysis with an error if "ModelPOSIX=true" is not set but the  'alpha.unix.Stream' checker is enabled? @Szelethus, maybe you have some insights with this?

The problem is, Dependencies and Weakdependencies both enforce a strict order of checker registration. What is discussed here is the following order:

1. StdCLibraryFunctionsChecker (modelling part)
2. StreamChecker
3. StdCLibraryFunctionArgsChecker (reporting part)

This falls apart because the modeling and reporting checkers are, in fact, the same checker in implementation. The "proper" solution would be to split up StdLibraryFunctions not in terms of a new checker entry in Checkers.td, but to split up the checker in implementation.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:786
   /// constraints (and different return value constraints).
-  const NoErrnoConstraint ErrnoIrrelevant{};
+  const NoErrnoConstraint ErrnoUnchanged{};
+  const ResetErrnoConstraint ErrnoIrrelevant{};
----------------
Hmm, do we need to specify this? Can't this be the default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247