klimek added a comment.
Also missing tests :)
================
Comment at: clangd/ClangdUnitStore.cpp:45
+ .first;
+ Result.RemovedFile = nullptr;
+ } else if (!compileCommandsAreEqual(It->second->getCompileCommand(),
----------------
ilya-biryukov wrote:
> klimek wrote:
> > Just say RemovedFile = nullptr in the struct?
> I'd argue that having it in the code, rather than declaration makes it more
> readable. Besides, default ctor of shared_ptr gives us nullptr anyway, so we
> can also leave it as is and remove the assignments altogether.
>
> I've moved `=nullptr` assignments out of the if/else bodies. Let me know if
> it still looks ugly.
Ah, I missed that it's a smart pointer; in that case, yes, remove the = nullptr.
================
Comment at: clangd/ClangdUnitStore.h:45-48
+ struct RecreateResult {
+ std::shared_ptr<CppFile> FileInCollection;
+ std::shared_ptr<CppFile> RemovedFile;
+ };
----------------
ilya-biryukov wrote:
> klimek wrote:
> > Not obvious to me what things in there mean.
> Added a comment. Hopefully makes sense now.
Better, thanks. Now, why does this need to be shared_ptr (as opposed to
unique_ptr)? Don't we always only have one?
https://reviews.llvm.org/D36398
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
