gamesh411 marked 8 inline comments as done.
gamesh411 added inline comments.


================
Comment at: 
clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:42
+BugType &
+SufficientSizeArrayIndexingChecker::GetBugTypeForType(const QualType T) const {
+  auto BT = BugTypeCache.find(T);
----------------
NoQ wrote:
> The whole point of bug types is to remain the same regardless of the specific 
> message. They're more like a category. Please use only one bug type for the 
> whole checker (unless you really find different categories of bugs).
I have removed the whole BugType factory think, that was really stupid in 
retrospect.


================
Comment at: 
clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:103
+  const auto *SuperSubRegion = dyn_cast<SubRegion>(SuperMemRegion);
+  // The checker has to access the extent of both the sub and the superregion.
+  if (!SuperSubRegion)
----------------
NoQ wrote:
> This deserves comments on what kinds of regions do you expect to see here. Do 
> i understand correctly that you expect `BaseMemRegion` to be an 
> `ElementRegion` and its superregion to be the whole array? 'Cause the former 
> is super unobvious and most likely //shouldn't// be that way.
I feel a bit more familiar now with the infrastructure now, and I used 
getDynamicElementCount as it now an available utility. Thanks for the comments, 
they were helpful for discovering how things work! :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69318



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to