[PATCH] D159474: [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies with auto

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.

Thanks, @Manna, these changes look good to me!



================
Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:175
     SmallVector<SymbolReference> Bases;
-    for (const auto BaseSpecifier : Decl->bases()) {
+    for (const auto &BaseSpecifier : Decl->bases()) {
       // skip classes not inherited as public
----------------
I agree that this looks like a good change. `CXXBaseSpecifier` contains a `SourceRange`, a `SourceLocation`, a `unsigned` (used as a bit-field, and a `TypeSourceInfo*`. Those are all individually cheap to copy, but together are large enough to justify use of a reference.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2256
 
-  for (auto [VD, Ignore] : FixItsForVariable) {
+  for (const auto &[VD, Ignore] : FixItsForVariable) {
     VarGrpRef Grp = VarGrpMgr.getGroupOfVar(VD);
----------------
This looks good too. The returned map value has `const VarDecl*` as a key and the element type is `FixItList` (aka `SmallVector<FixItHint, 4>`) . `FixItHint` contains two `CharSourceRange` objects, a `std::string`, and a `bool`, so we definitely don't want to be copying that; especially since the value isn't even wanted here!


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:903
   Symbols.emplace_back(std::move(*Class));
-  for (const auto Base : Record.Bases)
+  for (const auto &Base : Record.Bases)
     serializeRelationship(RelationshipKind::InheritsFrom, Record, Base);
----------------
Another case of `CXXBaseSpecifier` which, per earlier comments, is large enough to justify use of a reference.


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

https://reviews.llvm.org/D159474