[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501
   public:
+    const iterator &operator*() const { return *this; }
+
----------------
Is this just a dummy to make sure this satisfies some concepts? I think I comment might be helpful in that case, people might find a noop dereference operator confusing. Same for some other places.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:749-750
 
   referenced_vars_iterator referenced_vars_begin() const;
   referenced_vars_iterator referenced_vars_end() const;
+  llvm::iterator_range<referenced_vars_iterator> referenced_vars() const;
----------------
Do we need these with the addition of `referenced_vars`?


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:637-638
 
   region_iterator region_begin() const { return LiveRegionRoots.begin(); }
   region_iterator region_end() const { return LiveRegionRoots.end(); }
+  llvm::iterator_range<region_iterator> regions() const {
----------------
Should we remove these?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp:194-195
 
-      for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
-           ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
-        if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType())
+      for (const Expr *Arg : make_range(CallRec.AllocCall->arg_begin(),
+                                        CallRec.AllocCall->arg_end())) {
+        if (!Arg->getType()->isIntegralOrUnscopedEnumerationType())
----------------



================
Comment at: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp:54
+  for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+    delete Consumer;
   }
----------------
Hm, I wonder whether we actually want to use `unique_ptr`s to make the ownership explicit. Feel free to leave as is in this PR.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:173-177
+        } else {
         // The left-hand side may bind to a different value then the
         // computation type.
         LHSVal = svalBuilder.evalCast(Result, LTy, CTy);
+        }
----------------
Might be an issue with Phab, but indent looks strange to me here.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
-  for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
-                                               e = LE->capture_init_end();
-       i != e; ++i, ++CurField, ++Idx) {
+  for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+       ++i, ++CurField, ++Idx) {
----------------
Ha about using `capture_inits`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325