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?


cfe-commits mailing list

Reply via email to