[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 5 inline comments as done.
jkorous added inline comments.


================
Comment at: clangd/ClangdServer.cpp:529
+
+  WorkScheduler.runWithAST("CursorInfo", File, Bind(Action, std::move(CB)));
+}
----------------
sammccall wrote:
> sammccall wrote:
> > nit: SymbolInfo
> (This still says CursorInfo in the runWithAST call)
I sent my comments first and updated the diff a minute after. You are probably just too fast and saw the stale version :)


================
Comment at: clangd/XRefs.cpp:785
+    }
+    Results.emplace_back(std::move(NewSymbol));
+  }
----------------
sammccall wrote:
> jkorous wrote:
> > sammccall wrote:
> > > nit: push_back (and below)
> > Sure, no problem. I have to admit I thought these two are the same but I guess you pointed this out because there's a functional difference. I'd appreciate if you could advise where can I learn the details.
> Here the behavior is identical, so this is really a style thing.
> 
> `emplace_back` is a more powerful function, with semantics "make a new element": it will forward *any* argument list to a constructor of Symbol, selected by overload resolution.
> 
> `push_back` is a more constrained function, with semantics "insert this element": it takes one Symbol and forwards it to the move constructor (or copy constructor, as appropriate).
> 
> They're equivalent here because `emplace_back` can select the move-constructor when passed a single `Symbol&&`. But it communicates intent less clearly, so humans have a harder time (harder to understand the code) as does the compiler (error messages are worse).
Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54799/new/

https://reviews.llvm.org/D54799