[PATCH] D88297: [clangd] Trivial setter support when moving items to fields

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:461
+  if (auto *CE = llvm::dyn_cast<CallExpr>(RHS->IgnoreCasts())) {
+    // Make sure we get the version of move with 1 arg, the other is for moving
+    // ranges.
----------------
sammccall wrote:
> nit: you could skip this check if you like, the other variant isn't going to be on the RHS of an assignment :-)
You never know what crazy code people could concoct. It could be argued that this has a performance win by easily filtering out some bad candidates quickly and avoid needing to run the (slightly) more expensive checks on the name later down the line :-)


================
Comment at: clang-tools-extra/clangd/Hover.cpp:466
+    auto *ND = llvm::dyn_cast<NamedDecl>(CE->getCalleeDecl());
+    if (!ND)
+      return llvm::None;
----------------
kadircet wrote:
> nit: combine with the next condition, and perform the string comparison last, i.e.:
> 
> `if(!ND || !ND->isInStd || ND->getName() != "move")`
Can you explain the reasoning for moving the comparison to the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88297