[PATCH] D35894: Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle requested changes to this revision.
malaperle added a comment.
This revision now requires changes to proceed.

Normally we put [clangd] in the title.



================
Comment at: clangd/ClangdLSPServer.cpp:11
 #include "ClangdLSPServer.h"
+#include "clang/Basic/SourceManager.h"
 #include "JSONRPCDispatcher.h"
----------------
I don't think this include is needed?


================
Comment at: clangd/ClangdLSPServer.cpp:193
 
+
   std::string Completions;
----------------
unintentional change?


================
Comment at: clangd/ClangdLSPServer.cpp:209
 
-  auto Items = LangServer.Server.findDefinitions(
-      Params.textDocument.uri.file,
+  auto Items = LangServer.Server.findDefinitions(Params.textDocument.uri.file,
       Position{Params.position.line, Params.position.character}).Value;
----------------
unintentional change?


================
Comment at: clangd/ClangdServer.cpp:20
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
 #include <future>
----------------
is this include needed?


================
Comment at: clangd/ClangdServer.cpp:26
 
+const char* const DEFAULT_SOURCE_EXTENSIONS [] = { ".cpp", ".c", ".cc", ".cxx",
+    ".c++", ".C", ".m", ".mm" };
----------------
those line belong to another patch?


================
Comment at: clangd/ClangdServer.cpp:291
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  Units.runOnUnit(File, *FileContents.Draft, ResourceDir, CDB, PCHs,
-      TaggedFS.Value, [&](ClangdUnit &Unit) {
-        Result = Unit.findDefinitions(Pos);
-      });
+  Units.runOnUnit(
+      File, *FileContents.Draft, ResourceDir, CDB, PCHs, TaggedFS.Value,
----------------
unintentional reformat?


================
Comment at: clangd/ClangdServer.h:195
   /// Gets current document contents for \p File. \p File must point to a
-  /// currently tracked file.
+  /// currently tracked file
   /// FIXME(ibiryukov): This function is here to allow offset-to-Position
----------------
unintentional change?


================
Comment at: clangd/ClangdUnit.cpp:17
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:281
   DeclarationLocationsFinder(raw_ostream &OS,
-      const SourceLocation &SearchedLocation, ASTUnit &Unit) :
-      SearchedLocation(SearchedLocation), Unit(Unit) {}
+                             const SourceLocation &SearchedLocation,
+                             ASTUnit &Unit)
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:310
+
+    return SourceMgr.getFileOffset(SearchedLocation) == Offset &&
+           SourceMgr.getFileID(SearchedLocation) == FID;
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:314
 
-  void addDeclarationLocation(const SourceRange& ValSourceRange) {
-    const SourceManager& SourceMgr = Unit.getSourceManager();
-    const LangOptions& LangOpts = Unit.getLangOpts();
+  void addDeclarationLocation(const SourceRange &ValSourceRange) {
+    const SourceManager &SourceMgr = Unit.getSourceManager();
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:320
+                                                       0, SourceMgr, LangOpts);
+
+
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:345
       }
-      IdentifierInfo* IdentifierInfo = Result.getIdentifierInfo();
+      IdentifierInfo *IdentifierInfo = Result.getIdentifierInfo();
       if (IdentifierInfo && IdentifierInfo->hadMacroDefinition()) {
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:355
         MacroDefinition MacroDef =
-            Unit.getPreprocessor().getMacroDefinitionAtLoc(IdentifierInfo,
-                BeforeSearchedLocation);
-        MacroInfo* MacroInf = MacroDef.getMacroInfo();
+            Unit.getPreprocessor().getMacroDefinitionAtLoc(
+                IdentifierInfo, BeforeSearchedLocation);
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:390
   // Clang uses one-based numbers.
-  SourceLocation InputLocation = Unit->getLocation(FE, Pos.line + 1,
-      Pos.character + 1);
+  SourceLocation InputLocation =
+      Unit->getLocation(FE, Pos.line + 1, Pos.character + 1);
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.cpp:405
   // identifiers without another token in between.)
-  SourceLocation PeekBeforeLocation = Unit->getLocation(FE, Pos.line + 1,
-      Pos.character);
+  SourceLocation PeekBeforeLocation =
+      Unit->getLocation(FE, Pos.line + 1, Pos.character);
----------------
unintentional reformat?


================
Comment at: clangd/ClangdUnit.h:95
+  
 
   SourceLocation getBeginningOfIdentifier(const Position& Pos, const FileEntry* FE) const;
----------------
remove


================
Comment at: clangd/ProtocolHandlers.cpp:256
       "textDocument/completion",
+      // }
       llvm::make_unique<CompletionHandler>(Out, Callbacks));
----------------
remove?


================
Comment at: clangd/clients/clangd-vscode/.vscode/launch.json:6
+    { 
+        "name": "(gdb) Attach",
+        "type": "cppdbg",
----------------
remove?


================
Comment at: clangd/clients/clangd-vscode/src/extension.ts:58
 
+    clangdClient.trace = vscodejsonrpc.Trace.Verbose;
     const disposable = clangdClient.start();
----------------
Should be a different patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D35894