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

[Soumi Manna via Phabricator via cfe-commits]

Manna marked 7 inline comments as done.
Manna added inline comments.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:9426
       } else {
         for (auto IvarPair : DuplicateIvars) {
           Diag(IvarPair.first->getLocation(),
----------------
tahonermann wrote:
> 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.
Yes, i missed this case. I will investigate it further, so this case has been removed from the current patch.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1053
 
-  for (auto I : CleanedState->get<ObjectsUnderConstruction>()) {
+  for (const auto &I : CleanedState->get<ObjectsUnderConstruction>()) {
     if (SymbolRef Sym = I.second.getAsSymbol())
----------------
steakhal wrote:
> I think this is supposed to be just some lightweight handle: some discriminator and pointer under the hood. `sizeof(I)`
>  is just 40 bytes. Another point is that the copy construction is not customized, so the `sizeof` for such objects should be a good proxy for estimating how costly the copy is.
> 
> For me personally, if it fits into a cacheline (on `x86_64` I think its 64 bytes) (and ~trivially-copyable) then it should've probably taken by value.
> I haven't measured this theory, so take it with a pinch of salt.
Thanks for the explanation!


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

https://reviews.llvm.org/D148639