[NFC] Fix unnecessary copy with auto.

[steakhal]

I can only vouch for the SttaticAnalyzer portion.
Those parts makes sense to me.
However, I've seen cases where it doesn't resonate.
As a rule of thumb, if we are "copying" only at most 3 pointers, I'd rather still just make a copy instead of by reference.
Please check if your recommended changes reflect this.

If you decide to take it by reference, prefer taking it also by const.

Let me know if you oppose.

[srividya-sundaram]

Added 'const' type qualifiers per review comments.

[steakhal]

I went over the patch and I found only a single debatable case for taking by reference.
To clarify why I would prefer taking by value: value semantics is good for local reasoning; thus improves maintainability. We should only deviate from that when there is actual benefit for doing so.
Static analysis tools, such as Coverity, have false-positives. Some rules are better than others.
As a static analysis tool developer myself, I'd recommend carefully evaluating the findings before taking action for resolving them.
And if you find anything interesting, tool vendors are generally happy to receive feedback about their tool. I guess, they should as well understand that taking a pointer by reference doesn't improve anything.

clang/include/clang/Analysis/FlowSensitive/StorageLocation.h

129

This is only 2 pointers. I'd rather take them by value.

clang/lib/AST/Interp/ByteCodeEmitter.cpp

64

Only 2 pointers.

clang/lib/AST/RecordLayoutBuilder.cpp

3728

It's 24 bytes, which is like 3 pointers.

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

38

2 pointers.

86

2 pointers.

clang/lib/Sema/SemaRISCVVectorLookup.cpp

305

It's just a single long. 8 bytes.
Taking by ref would definitely not help the situation.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2621

8 + 16 bytes. Basically 3 pointers.

clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

851–853

size_t and a pointer. 16 bytes in total.

clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

534

8 + 16 bytes.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2821

8 + 16 bytes.

3081

8 + 16 bytes.

3558

8 + 16 bytes.

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

945

16 + 24 bytes. Okay, this is basically the first case where it actually makes sense to take by ref.
But we should still prefer const ref over ref

1373

8 + 16 bytes.

clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp

365

8 + 4 bytes.

clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp

154

8 + 4 bytes.

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

169

16 bytes.

clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

166

8 + 4 bytes.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

1196–1198

Here we are talking about some generator of some pointers and indexes.
I don't think we would benefit from taking this by ref - even though in total it's around 48 bytes.

[tahonermann]

I agree with the analysis done by @steakhal. The static analyzer being used (which I'm not allowed to name) is being too noisy in these cases; it complains about every case where an object of class type is copied regardless of size. An issue has been reported to the static analysis vendor about this. These complaints from the static analyzer will need to be moderated until the vendor satisfactorily addresses the reported issue.

[srividya-sundaram]

In D157129#4564766, @steakhal wrote:

I went over the patch and I found only a single debatable case for taking by reference.
To clarify why I would prefer taking by value: value semantics is good for local reasoning; thus improves maintainability. We should only deviate from that when there is actual benefit for doing so.
Static analysis tools, such as Coverity, have false-positives. Some rules are better than others.
As a static analysis tool developer myself, I'd recommend carefully evaluating the findings before taking action for resolving them.
And if you find anything interesting, tool vendors are generally happy to receive feedback about their tool. I guess, they should as well understand that taking a pointer by reference doesn't improve anything.

Thank you, @steakhal for the detailed feedback.
Based on your comments, and after discussing with @tahonermann, I am abandoning this patch.

[aaron.ballman]

In D157129#4571112, @srividya-sundaram wrote:

Based on your comments, and after discussing with @tahonermann, I am abandoning this patch.

Btw, it helps reviewers if you explicitly abandon the patch in Phabricator. There's a drop-down menu that says "Add Action..." and you can choose to abandon the review from there.

[srividya-sundaram]

I did spend a good amount of time trying to figure out how to abandon this review and gave up eventually.
Even after your comment, I had to search a bit for the "Add Action.." drop down. Clearly, I'm UI challenged.

clang/include/clang/Analysis/FlowSensitive/StorageLocation.h

public:
   StorageLocation *getChild(const ValueDecl &D) const {
     auto It = Children.find(&D);
     LLVM_DEBUG({
       if (It == Children.end()) {
         llvm::dbgs() << "Couldn't find child " << D.getNameAsString()
                      << " on StorageLocation " << this << " of type "
                      << getType() << "\n";
         llvm::dbgs() << "Existing children:\n";
-        for ([[maybe_unused]] auto [Field, Loc] : Children) {
+        for ([[maybe_unused]] auto &[Field, Loc] : Children) {
           llvm::dbgs() << Field->getNameAsString() << "\n";
         }
       }
     });
     assert(It != Children.end());
     return It->second;
   }
 

clang/lib/AST/Interp/ByteCodeEmitter.cpp

if (const auto *MD = dyn_cast<CXXMethodDecl>(FuncDecl)) {
     // Set up lambda capture to closure record field mapping.
     if (isLambdaCallOperator(MD)) {
       const Record *R = P.getOrCreateRecord(MD->getParent());
       llvm::DenseMap<const ValueDecl *, FieldDecl *> LC;
       FieldDecl *LTC;
 
       MD->getParent()->getCaptureFields(LC, LTC);
 
-      for (auto Cap : LC) {
+      for (auto &Cap : LC) {
         unsigned Offset = R->getField(Cap.second)->Offset;
         this->LambdaCaptures[Cap.first] = {
             Offset, Cap.second->getType()->isReferenceType()};
       }
       // FIXME: LambdaThisCapture
       (void)LTC;
     }
   }

clang/lib/AST/RecordLayoutBuilder.cpp

if (!isMsLayout(*this))
     OS << "  DataSize:" << toBits(Info.getDataSize()) << "\n";
   OS << "  Alignment:" << toBits(Info.getAlignment()) << "\n";
   if (Target->defaultsToAIXPowerAlignment())
     OS << "  PreferredAlignment:" << toBits(Info.getPreferredAlignment())
        << "\n";
   if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
     OS << "  BaseOffsets: [";
     const CXXRecordDecl *Base = nullptr;
-    for (auto I : CXXRD->bases()) {
+    for (auto &I : CXXRD->bases()) {
       if (I.isVirtual())
         continue;
       if (Base)
         OS << ", ";
       Base = I.getType()->getAsCXXRecordDecl();
       OS << Info.CXXInfo->BaseOffsets[Base].getQuantity();
     }
     OS << "]>\n";

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

void clang::dataflow::copyRecord(RecordStorageLocation &Src,
   LLVM_DEBUG({
     if (!compatibleTypes) {
       llvm::dbgs() << "Source type " << Src.getType() << "\n";
       llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
     }
   });
   assert(compatibleTypes);
 
-  for (auto [Field, DstFieldLoc] : Dst.children()) {
+  for (auto &[Field, DstFieldLoc] : Dst.children()) {
     StorageLocation *SrcFieldLoc = Src.getChild(*Field);
 
     assert(Field->getType()->isReferenceType() ||
            (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
 
     if (Field->getType()->isRecordType()) {
       copyRecord(cast<RecordStorageLocation>(*SrcFieldLoc),
                  cast<RecordStorageLocation>(*DstFieldLoc), Env);
if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
         Loc1.getType().getCanonicalType().getUnqualifiedType()) {
       llvm::dbgs() << "Loc1 type " << Loc1.getType() << "\n";
       llvm::dbgs() << "Loc2 type " << Loc2.getType() << "\n";
     }
   });
   assert(Loc2.getType().getCanonicalType().getUnqualifiedType() ==
          Loc1.getType().getCanonicalType().getUnqualifiedType());
 
-  for (auto [Field, FieldLoc1] : Loc1.children()) {
+  for (auto &[Field, FieldLoc1] : Loc1.children()) {
     StorageLocation *FieldLoc2 = Loc2.getChild(*Field);
 
     assert(Field->getType()->isReferenceType() ||
            (FieldLoc1 != nullptr && FieldLoc2 != nullptr));
 
     if (Field->getType()->isRecordType()) {
       if (!recordsEqual(cast<RecordStorageLocation>(*FieldLoc1), Env1,
                         cast<RecordStorageLocation>(*FieldLoc2), Env2))

clang/lib/Sema/SemaRISCVVectorLookup.cpp

for (unsigned int TypeRangeMaskShift = 0;
         // Create masked intrinsic.
         std::optional<RVVTypes> MaskTypes =
             TypeCache.computeTypes(BaseType, Log2LMUL, Record.NF, ProtoMaskSeq);
         InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr, true,
                          *MaskTypes, MaskedHasPolicy, DefaultPolicy);
         if (Record.MaskedPolicyScheme == PolicyScheme::SchemeNone)
           continue;
         // Create masked policy intrinsic.
-        for (auto P : SupportedMaskedPolicies) {
+        for (auto &P : SupportedMaskedPolicies) {
           llvm::SmallVector<PrototypeDescriptor> PolicyPrototype =
               RVVIntrinsic::computeBuiltinTypes(
                   BasicProtoSeq, /*IsMasked=*/true, Record.HasMaskedOffOperand,
                   Record.HasVL, Record.NF, MaskedPolicyScheme, P,
                   Record.IsTuple);
           std::optional<RVVTypes> PolicyTypes = TypeCache.computeTypes(
               BaseType, Log2LMUL, Record.NF, PolicyPrototype);
           InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr,

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

 void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
     CheckerContext &C) const {
   ProgramStateRef state = C.getState();
   CStringLengthTy Entries = state->get<CStringLength>();
   if (Entries.isEmpty())
     return;
 
   CStringLengthTy::Factory &F = state->get_context<CStringLength>();
-  for (auto [Reg, Len] : Entries) {
+  for (auto &[Reg, Len] : Entries) {
     if (SymbolRef Sym = Len.getAsSymbol()) {
       if (SR.isDead(Sym))
         Entries = F.remove(Entries, Reg);
     }
   }
 
   state = state->set<CStringLength>(Entries);
   C.addTransition(state);

clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

if (argumentNumber < 0) {
       OD = OD->getSuperClass();
       argumentNumber = getLocalizedArgumentForSelector(OD->getIdentifier(), S);
     }
   }
 
   if (argumentNumber < 0) { // There was no match in UIMethods
     if (const Decl *D = msg.getDecl()) {
       if (const ObjCMethodDecl *OMD = dyn_cast_or_null<ObjCMethodDecl>(D)) {
-        for (auto [Idx, FormalParam] : llvm::enumerate(OMD->parameters())) {
+        for (const auto &[Idx, FormalParam] :
+             llvm::enumerate(OMD->parameters())) {
           if (isAnnotatedAsTakingLocalized(FormalParam)) {
             argumentNumber = Idx;
             break;
           }
         }
       }
     }
   }
 

clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

bool ErrorIsReturned = (OpCode == BO_EQ && RHS != NoErr) ||
                            (OpCode == BO_NE && RHS == NoErr);
     if (!Assumption)
       ErrorIsReturned = !ErrorIsReturned;
     if (ErrorIsReturned)
       ReturnSymbol = SIE->getLHS();
   }
 
   if (ReturnSymbol)
-    for (auto [Sym, AllocState] : AMap) {
+    for (auto &[Sym, AllocState] : AMap) {
       if (ReturnSymbol == AllocState.Region)
         State = State->remove<AllocatedData>(Sym);
     }
 
   return State;
 }
 
 void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR,

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

assert(state->get<ReallocPairs>() ==
            C.getState()->get<ReallocPairs>());
     assert(state->get<FreeReturnValue>() ==
            C.getState()->get<FreeReturnValue>());
     return;
   }
 
   // Cleanup the Realloc Pairs Map.
   ReallocPairsTy RP = state->get<ReallocPairs>();
-  for (auto [Sym, ReallocPair] : RP) {
+  for (auto &[Sym, ReallocPair] : RP) {
     if (SymReaper.isDead(Sym) || SymReaper.isDead(ReallocPair.ReallocatedSym)) {
       state = state->remove<ReallocPairs>(Sym);
     }
   }
 
   // Cleanup the FreeReturnValue Map.
   FreeReturnValueTy FR = state->get<FreeReturnValue>();
   for (auto [Sym, RetSym] : FR) {
for (SymbolRef Sym : llvm::make_first_range(RS)) {
     ConditionTruthVal AllocFailed = CMgr.isNull(state, Sym);
     if (AllocFailed.isConstrainedTrue())
       state = state->remove<RegionState>(Sym);
   }
 
   // Realloc returns 0 when reallocation fails, which means that we should
   // restore the state of the pointer being reallocated.
   ReallocPairsTy RP = state->get<ReallocPairs>();
-  for (auto [Sym, ReallocPair] : RP) {
+  for (auto &[Sym, ReallocPair] : RP) {
     // If the symbol is assumed to be NULL, remove it from consideration.
     ConstraintManager &CMgr = state->getConstraintManager();
     ConditionTruthVal AllocFailed = CMgr.isNull(state, Sym);
     if (!AllocFailed.isConstrainedTrue())
       continue;
 
     SymbolRef ReallocSym = ReallocPair.ReallocatedSym;
     if (const RefState *RS = state->get<RegionState>(ReallocSym)) {

 
 void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State,
                                const char *NL, const char *Sep) const {
 
   RegionStateTy RS = State->get<RegionState>();
 
   if (!RS.isEmpty()) {
     Out << Sep << "MallocChecker :" << NL;
-    for (auto [Sym, Data] : RS) {
+    for (auto &[Sym, Data] : RS) {
       const RefState *RefS = State->get<RegionState>(Sym);
       AllocationFamily Family = RefS->getAllocationFamily();
       std::optional<MallocChecker::CheckKind> CheckKind =
           getCheckIfTracked(Family);
       if (!CheckKind)
         CheckKind = getCheckIfTracked(Family, true);
 
       Sym->dumpToStream(Out);

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

 
 // The return value of a property access is typically a temporary value which
 // will not be tracked in a persistent manner by the analyzer.  We use
 // evalAssume() in order to immediately record constraints on those temporaries
 // at the time they are imposed (e.g. by a nil-check conditional).
 ProgramStateRef NullabilityChecker::evalAssume(ProgramStateRef State, SVal Cond,
                                                bool Assumption) const {
   PropertyAccessesMapTy PropertyAccesses = State->get<PropertyAccessesMap>();
-  for (auto [PropKey, PropVal] : PropertyAccesses) {
+  for (auto &[PropKey, PropVal] : PropertyAccesses) {
     if (!PropVal.isConstrainedNonnull) {
       ConditionTruthVal IsNonNull = State->isNonNull(PropVal.Value);
       if (IsNonNull.isConstrainedTrue()) {
         ConstrainedPropertyVal Replacement = PropVal;
         Replacement.isConstrainedNonnull = true;
         State = State->set<PropertyAccessesMap>(PropKey, Replacement);
       } else if (IsNonNull.isConstrainedFalse()) {
         // Space optimization: no point in tracking constrained-null cases
Out << Sep << NL
         << "Nullability invariant was violated, warnings suppressed." << NL;
 
   if (B.isEmpty())
     return;
 
   if (!State->get<InvariantViolated>())
     Out << Sep << NL;
 
-  for (auto [Region, State] : B) {
+  for (auto &[Region, State] : B) {
     Out << Region << " : ";
     State.print(Out);
     Out << NL;
   }
 }
 
 void ento::registerNullabilityBase(CheckerManager &mgr) {
   mgr.registerChecker<NullabilityChecker>();

clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp

if (PreCallFlags != SelfFlag_None) {
     }
     if (PreCallFlags & SelfFlag_InitRes) {
       Out << "  An argument of the current call came from an init method."
           << NL;
     }
   }
 
   Out << NL;
-  for (auto [Sym, Flag] : FlagMap) {
+  for (auto &[Sym, Flag] : FlagMap) {
     Out << Sym << " : ";
 
     if (Flag == SelfFlag_None)
       Out << "none";
 
     if (Flag & SelfFlag_Self)
       Out << "self variable";
 

clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp

static void checkObjCUnusedIvar(const ObjCImplementationDecl *D,
   // for functions inside the @implementation that reference these ivars.
   // FIXME: In the future hopefully we can just use the lexical DeclContext
   // to go from the ObjCImplementationDecl to the lexically "nested"
   // C functions.
   const SourceManager &SM = BR.getSourceManager();
   Scan(M, D->getDeclContext(), SM.getFileID(D->getLocation()), SM);
 
   // Find ivars that are unused.
-  for (auto [Ivar, State] : M)
+  for (auto &[Ivar, State] : M)
     if (State == Unused) {
       std::string sbuf;
       llvm::raw_string_ostream os(sbuf);
       os << "Instance variable '" << *Ivar << "' in class '" << *ID
          << "' is never used by the methods in its @implementation "
             "(although it may be used by category methods).";
 
       PathDiagnosticLocation L =

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

void RetainCountChecker::checkPostStmt(const BlockExpr *BE,
 
   // FIXME: For now we invalidate the tracking of all symbols passed to blocks
   // via captured variables, even though captured variables result in a copy
   // and in implicit increment/decrement of a retain count.
   SmallVector<const MemRegion*, 10> Regions;
   const LocationContext *LC = C.getLocationContext();
   MemRegionManager &MemMgr = C.getSValBuilder().getRegionManager();
 
-  for (auto Var : ReferencedVars) {
+  for (auto &Var : ReferencedVars) {
     const VarRegion *VR = Var.getCapturedRegion();
     if (VR->getSuperRegion() == R) {
       VR = MemMgr.getVarRegion(VR->getDecl(), LC);
     }
     Regions.push_back(VR);
   }
 
   state = state->scanReachableSymbols<StopTrackingCallback>(Regions).getState();

clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

static bool isLeaked(SymbolRef Sym, const StreamState &SS,
   return false;
 }
 
 void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                            CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SymbolVector LeakedStreams;
   StreamMapTy TrackedStreams = State->get<StreamMap>();
-  for (auto [Sym, StreamStatus] : TrackedStreams) {
+  for (auto &[Sym, StreamStatus] : TrackedStreams) {
     bool IsSymDead = SymReaper.isDead(Sym);
 
     // Collect leaked symbols.
     if (isLeaked(Sym, StreamStatus, IsSymDead, State))
       LeakedStreams.push_back(Sym);
 
     // Remove the dead symbol from the streams map.
     if (IsSymDead)

clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp

bool StackAddrEscapeChecker::isSemaphoreCaptured(const BlockDecl &B) const {
   }
   return false;
 }
 
 SmallVector<const MemRegion *, 4>
 StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
                                                 CheckerContext &C) {
   SmallVector<const MemRegion *, 4> Regions;
-  for (auto Var : B.referenced_vars()) {
+  for (auto &Var : B.referenced_vars()) {
     SVal Val = C.getState()->getSVal(Var.getCapturedRegion());
     const MemRegion *Region = Val.getAsRegion();
     if (Region && isa<StackSpaceRegion>(Region->getMemorySpace()))
       Regions.push_back(Region);
   }
   return Regions;
 }
 

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

void StdLibraryFunctionsChecker::appendOutOfRangeDesc(llvm::APSInt RMin,
   }
 }
 
 void StdLibraryFunctionsChecker::RangeConstraint::applyOnWithinRange(
     BasicValueFactory &BVF, QualType ArgT, const RangeApplyFunction &F) const {
   if (Ranges.empty())
     return;
 
-  for (auto [Start, End] : getRanges()) {
+  for (auto &[Start, End] : getRanges()) {
     const llvm::APSInt &Min = BVF.getValue(Start, ArgT);
     const llvm::APSInt &Max = BVF.getValue(End, ArgT);
     assert(Min <= Max);
     if (!F(Min, Max))
       return;
   }
 }
 
bool StdLibraryFunctionsChecker::Signature::matches(
   // Check the return type.
   if (!isIrrelevant(RetTy)) {
     QualType FDRetTy = RemoveRestrict(FD->getReturnType().getCanonicalType());
     if (RetTy != FDRetTy)
       return false;
   }
 
   // Check the argument types.
-  for (auto [Idx, ArgTy] : llvm::enumerate(ArgTys)) {
+  for (const auto &[Idx, ArgTy] : llvm::enumerate(ArgTys)) {
     if (isIrrelevant(ArgTy))
       continue;
     QualType FDArgTy =
         RemoveRestrict(FD->getParamDecl(Idx)->getType().getCanonicalType());
     if (ArgTy != FDArgTy)
       return false;
   }
 

clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp

 UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE,
                                             CheckerContext &C) const {
   if (!BE->getBlockDecl()->hasCaptures())
     return;
 
   ProgramStateRef state = C.getState();
   auto *R = cast<BlockDataRegion>(C.getSVal(BE).getAsRegion());
 
-  for (auto Var : R->referenced_vars()) {
+  for (auto &Var : R->referenced_vars()) {
     // This VarRegion is the region associated with the block; we need
     // the one associated with the encompassing context.
     const VarRegion *VR = Var.getCapturedRegion();
     const VarDecl *VD = VR->getDecl();
 
     if (VD->hasAttr<BlocksAttr>() || !VD->hasLocalStorage())
       continue;
 

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

void ExprEngine::VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred,
   // If we created a new MemRegion for the block, we should explicitly bind
   // the captured variables.
   if (const BlockDataRegion *BDR =
       dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) {
 
     auto ReferencedVars = BDR->referenced_vars();
     auto CI = BD->capture_begin();
     auto CE = BD->capture_end();
-    for (auto Var : ReferencedVars) {
+    for (auto &Var : ReferencedVars) {
       const VarRegion *capturedR = Var.getCapturedRegion();
       const TypedValueRegion *originalR = Var.getOriginalRegion();
 
       // If the capture had a copy expression, use the result of evaluating
       // that expression, otherwise use the original value.
       // We rely on the invariant that the block declaration's capture variables
       // are a prefix of the BlockDataRegion's referenced vars (which may include
       // referenced globals, etc.) to enable fast lookup of the capture for a

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
   const MemRegion *R = svalBuilder.getRegionManager().getCXXTempObjectRegion(
       LE, LocCtxt);
   SVal V = loc::MemRegionVal(R);
 
   ProgramStateRef State = Pred->getState();
 
   // If we created a new MemRegion for the lambda, we should explicitly bind
   // the captures.
-  for (auto const [Idx, FieldForCapture, InitExpr] :
+  for (auto const &[Idx, FieldForCapture, InitExpr] :
        llvm::zip(llvm::seq<unsigned>(0, -1), LE->getLambdaClass()->fields(),
                  LE->capture_inits())) {
     SVal FieldLoc = State->getLValue(FieldForCapture, V);
 
     SVal InitVal;
     if (!FieldForCapture->hasCapturedVLAType()) {
       assert(InitExpr && "Capture missing initialization expression");
 
       // Capturing a 0 length array is a no-op, so we ignore it to get a more
       // accurate analysis. If it's not ignored, it would set the default

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

 
 void BlockCodeRegion::dumpToStream(raw_ostream &os) const {
   os << "block_code{" << static_cast<const void *>(this) << '}';
 }
 
 void BlockDataRegion::dumpToStream(raw_ostream &os) const {
   os << "block_data{" << BC;
   os << "; ";
-  for (auto Var : referenced_vars())
+  for (auto &Var : referenced_vars())
     os << "(" << Var.getCapturedRegion() << "<-" << Var.getOriginalRegion()
        << ") ";
   os << '}';
 }
 
 void CompoundLiteralRegion::dumpToStream(raw_ostream &os) const {
   // FIXME: More elaborate pretty-printing.
   os << "{ S" << CL->getID(getContext()) <<  " }";
getStackOrCaptureRegionForDeclContext(const LocationContext *LC,
   while (LC) {
     if (const auto *SFC = dyn_cast<StackFrameContext>(LC)) {
       if (cast<DeclContext>(SFC->getDecl()) == DC)
         return SFC;
     }
     if (const auto *BC = dyn_cast<BlockInvocationContext>(LC)) {
       const auto *BR = static_cast<const BlockDataRegion *>(BC->getData());
       // FIXME: This can be made more efficient.
-      for (auto Var : BR->referenced_vars()) {
+      for (auto &Var : BR->referenced_vars()) {
         const TypedValueRegion *OrigR = Var.getOriginalRegion();
         if (const auto *VR = dyn_cast<VarRegion>(OrigR)) {
           if (VR->getDecl() == VD)
             return cast<VarRegion>(Var.getCapturedRegion());
         }
       }
     }
 

clang/lib/StaticAnalyzer/Core/ProgramState.cpp

if (isa<MemSpaceRegion>(Super)) {
       StoreManager &StoreMgr = state->getStateManager().getStoreManager();
       if (!StoreMgr.scanReachableSymbols(state->getStore(), SR, *this))
         return false;
     }
   }
 
   // Regions captured by a block are also implicitly reachable.
   if (const BlockDataRegion *BDR = dyn_cast<BlockDataRegion>(R)) {
-    for (auto Var : BDR->referenced_vars()) {
+    for (auto &Var : BDR->referenced_vars()) {
       if (!scan(Var.getCapturedRegion()))
         return false;
     }
   }
 
   return true;
 }
 

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

if (const auto *RD = TO->getValueType()->getAsCXXRecordDecl()) {
         }
       }
     }
   }
 
   // BlockDataRegion?  If so, invalidate captured variables that are passed
   // by reference.
   if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(baseR)) {
-    for (auto Var : BR->referenced_vars()) {
+    for (auto &Var : BR->referenced_vars()) {
       const VarRegion *VR = Var.getCapturedRegion();
       const VarDecl *VD = VR->getDecl();
       if (VD->hasAttr<BlocksAttr>() || !VD->hasLocalStorage()) {
         AddToWorkList(VR);
       }
       else if (Loc::isLocType(VR->getValueType())) {
         // Map the current bindings to a Store to retrieve the value
         // of the binding.  If that binding itself is a region, we should
void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
 
   // If V is a region, then add it to the worklist.
   if (const MemRegion *R = V.getAsRegion()) {
     AddToWorkList(R);
     SymReaper.markLive(R);
 
     // All regions captured by a block are also live.
     if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) {
-      for (auto Var : BR->referenced_vars())
+      for (auto &Var : BR->referenced_vars())
         AddToWorkList(Var.getCapturedRegion());
     }
   }
 
 
   // Update the set of live symbols.
   for (SymbolRef Sym : V.symbols())
     SymReaper.markLive(Sym);