ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM and a few NITs.
================
Comment at: clangd/index/dex/Iterator.cpp:376
+ }
+ default:
+ RealChildren.push_back(std::move(Child));
----------------
Maybe replace with `case Kind::Other` to make sure compiler gives a warning to
update the switch when new kinds are added?
Same for other switches on kind
================
Comment at: clangd/index/dex/Iterator.cpp:378
+ RealChildren.push_back(std::move(Child));
+ }
+ switch (RealChildren.size()) {
----------------
Add braces for `for` statement to make sure there's a closing brace at the
right indentation?
================
Comment at: clangd/index/dex/Iterator.cpp:385
+ default:
+ return llvm::make_unique<AndIterator>(move(RealChildren));
+ }
----------------
Maybe use the qualified `std::move` for consistency with the previous line?
================
Comment at: clangd/index/dex/Iterator.cpp:407
+ RealChildren.push_back(std::move(Child));
+ }
+ switch (RealChildren.size()) {
----------------
Again, `for () switch` pattern ends up putting the closing brace at the wrong
indent level. Consider adding the braces around for the loop body.
================
Comment at: clangd/index/dex/Iterator.cpp:414
+ default:
+ return llvm::make_unique<OrIterator>(move(RealChildren));
+ }
----------------
Maybe use the qualified `std::move` for consistency with the previous line?
================
Comment at: clangd/index/dex/Iterator.h:109
private:
+ Kind MyKind;
virtual llvm::raw_ostream &dump(llvm::raw_ostream &OS) const = 0;
----------------
Maybe put data members after functions to follow the pattern we have in most
(all?) other places in clangd?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52789
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
