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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
