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