[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

Balázs Benics via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 1 02:02:12 PDT 2020


steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:52
+  ///   Offset: SymIntExpr{conj{n, int}, +, 12, long long}
+  class RawOffsetCalculator final
+      : public MemRegionVisitor<RawOffsetCalculator, RegionRawOffsetV2> {
----------------
martong wrote:
> Since you are already deep in refactoring. This calculation together with the enclosing class seems to be useful for other Checkers too. Would make sense to put this to `CheckerHelpers`?
> E.g. the PlacementNewChecker could have benefited from this class if it had been available for use during the implementation of the checker.
You are probably right.
However, I'm not qualified enough to make such a decision.

By the same token, I don't really understand why would anyone do such //unfolding// by hand?
Could someone clarify the use-cases for `RegionRawOffset` and `RegionRawOffsetV2`? @NoQ @vsavchenko @Szelethus?
Eg. we don't use such facilities in the `evalBindOpL#` functions, but it would probably come handy.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:171-172
 
-  SVal extentBegin = computeExtentBegin(svalBuilder, rawOffset.getRegion());
+      llvm::APSInt RHSConstant =
+          APSIntType(Index.getValue()).convert(E->getRHS());
 
----------------
Here we 'c-style' integral cast the right-hand side value to match the signess and bitwidth of `Index`. @martong 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86873



More information about the cfe-commits mailing list