[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:202-203
   // CHECK UPPER BOUND
-  DefinedOrUnknownSVal Size = getDynamicExtent(state, Reg, svalBuilder);
+  SVal Size = svalBuilder.convertToArrayIndex(
+      getDynamicExtent(state, Reg, svalBuilder));
   if (auto KnownSize = Size.getAs<NonLoc>()) {
----------------
donat.nagy wrote:
> I wonder whether it would be better to move this conversion into the definition of `getDynamicExtent` to ensure that it has a consistent return type. Are there any situations where it's useful that `getDynamicExtent` can return something that's not an `ArrayIndexTy`?
Use consistent return type is necessary. The problem is which version should we choose.
I didn't touch this before I can get a clear selection.

At first I tended to think that all extent/count should be unsigned, but quickly dropped this idea
since `getDynamicExtentWithOffset` gives reasonable note that extent/offset (even count) can
be negative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707