martong added a comment. In D75063#1923780 <https://reviews.llvm.org/D75063#1923780>, @NoQ wrote:
> This is basically a shorthand for "outside [0, 0]", right? I don't mind ^.^ Yeah, and my first attempt was exactly to implement this with ranges. However, it failed when I realized that we cannot cast a pointer to `NonLoc`, so the already written `RangeConstraint::apply*` functions could not work (I would have to add another branch for handling `Loc` kind of SVals for the pointer case). ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:175 + using ValueConstraint::ValueConstraint; + bool CannotBeNull = true; + ---------------- Szelethus wrote: > martong wrote: > > Szelethus wrote: > > > What does this do? Is it ever used in the patch? > > Yes, it is used. We use it in `apply` the value is passed to `assume`. > > And in `negate` we flip the value. > Forgot my eyes in the office. Woops. I would still prefer a line of comment > here :) Ok, I added a comment about its role. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:684 + .Case({ + ReturnValueCondition(LessThanOrEq, ArgNo(2)), + }) ---------------- martong wrote: > steakhal wrote: > > Two lines below you are using the `{0U}` initialization syntax, and here > > the simple constructor call syntax. > > Shouldn't we pick one? > Yes, definitely. I think I am going to use brace initialization syntax > everywhere. Finally I ended up with the parens. :) ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:889-890 Read(LongLongTy, LongLongMax)}}, {"fread", Summaries{Fread()}}, - {"fwrite", Summaries{Fread()}}, + {"fwrite", Summaries{Fwrite()}}, // getline()-like functions either fail or read at least the delimiter. ---------------- Szelethus wrote: > Not super relevant to this specific revision, but shouldn't we leave these to > `StreamChecker`? Well, yeah we could remove `fread` and `fwrite` from the summaries entirely at some point, but that will require changing the test files here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75063/new/ https://reviews.llvm.org/D75063 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits