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

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:34
+    if (auto SSize =
+            SVB.convertToArrayIndex(*Size).getAs<DefinedOrUnknownSVal>())
+      return *SSize;
----------------
danix800 wrote:
> donat.nagy wrote:
> > I think it's a good convention if `getDynamicExtent()` will always return concrete values as `ArrayIndexTy`. (If I didn't miss some very obscure case, then this will be true when this commit is merged.) 
> Could you elaborate this a bit more please? Do you mean by `nonloc::ConcreteInt` rather than `DefinedOrUnknownSVal`?
Sorry for the confusing comment! I just intended to summarize what you did without requesting any additional change.

My remark highlighted that (after your change) whenever the result of this function is a `nonloc::ConcreteInt`, the `getType()` method of that `ConcreteInt` object will return `ArrayIndexTy` (which is a `QualType` constant that represents the type `long long`).

There are also some situations when the result of `getDynamicExtent()` is not a `nonloc::ConcreteInt`; so it's correct that the return type should be `DefinedOrUnknownSVal` (which is the common supertype of all possible results [1]) and it's correct to use this return type in the `getAs<>` on the highlighted line.

[1] See inheritance diagram at https://clang.llvm.org/doxygen/classclang_1_1ento_1_1SVal.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707