[PATCH] D42173: [clangd] Simplify code handling compile commands

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 22 01:41:21 PST 2018


sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clangd/ClangdUnit.h:62
+/// Information required to run clang (e.g., to parse AST or do code
+/// completion).
+struct ParseInputs {
----------------
sammccall wrote:
> unwrap?
This is not done. Is clang-format doing this?

(It looks like exactly 80 cols, but in any case the second comma shouldn't be there :-))


================
Comment at: clangd/ClangdUnit.h:64
+struct ParseInputs {
+  ParseInputs(tooling::CompileCommand CompileCommand,
+              IntrusiveRefCntPtr<vfs::FileSystem> FS, std::string Contents);
----------------
remove the constructor and just use brace-init?


================
Comment at: clangd/ClangdUnit.h:226
+  /// cancelRebuild().
+  llvm::Optional<tooling::CompileCommand> getLastCommand() const;
 
----------------
I think that this might actually be the best place to document the invariant you rely on elsewhere (despite it being a layering violation).

  // In practice we always call rebuild() when adding a CppFile to the collection,
  // and only `cancelRebuild()` after removing it. This means files in the CppFileCollection
  // always have a compile command available.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42173





More information about the cfe-commits mailing list