[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

After seeing this review, I filed a support case with Synopsys with a request to stop reporting `AUTO_CAUSES_COPY` issues for small objects of class types, at least in the absence of additional evidence that a reference would be preferred.



================
Comment at: clang/lib/AST/ODRHash.cpp:597
   auto Bases = Record->bases();
-  for (auto Base : Bases) {
+  for (const auto &Base : Bases) {
     AddQualType(Base.getType());
----------------
This seems like a good change; `CXXBaseSpecifier` isn't particularly large, but this seems like a case where reference semantics are more intuitive anyway.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:9426
       } else {
         for (auto IvarPair : DuplicateIvars) {
           Diag(IvarPair.first->getLocation(),
----------------
aaron.ballman wrote:
> Why is the above considered too expensive but this one is not?
I'm guessing that Coverity reported it as a second occurrence and those are easy to overlook.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:989
   std::vector<FileID> Keys;
-  for (auto F : Files)
+  for (const auto &F : Files)
     Keys.push_back(F.first);
----------------
aaron.ballman wrote:
> `FileID` is an `int`, not expensive to copy.
`Files` has type `llvm::DenseMap<FileID, MarkedFile>`, so the element type is `DenseMapPair<FileID, MarkedFile>`. `MarkedFile` contains a couple of vectors, so this looks like a win to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148639