[clangd] Trivial setter support when moving items to fields

[njames93]

Summary

Extend the Trivial setter documentation to support cases where the value is moved into a field using std::move.

[sammccall]

Nice, thanks!

clang-tools-extra/clangd/Hover.cpp

463

nit: you could skip this check if you like, the other variant isn't going to be on the RHS of an assignment :-)

470

I guess you want !ND->getIdentifier() || ND->getName() != "move" || ... to guard against it being a special name somehow - getName() asserts in that case.

[kadircet]

clang-tools-extra/clangd/Hover.cpp

414

can you also update the docs?

468

nit: combine with the next condition, and perform the string comparison last, i.e.:

if(!ND || !ND->isInStd || ND->getName() != "move")

[njames93]

Updated function doc
Fix potential assertion

clang-tools-extra/clangd/Hover.cpp

463

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 :-)

468

Can you explain the reasoning for moving the comparison to the end?

[kadircet]

clang-tools-extra/clangd/Hover.cpp

468

it was to skip a potentialyl expensive string comparison, but now that I look into isInStdNamespace it is also performing a string comparison in the end. so nvm.

clang-tools-extra/clangd/Hover.cpp

const auto *OnlyReturn = (Body && Body->size() == 1)
                                ? llvm::dyn_cast<ReturnStmt>(Body->body_front())
                                : nullptr;
   if (!OnlyReturn || !OnlyReturn->getRetValue())
     return llvm::None;
   return fieldName(OnlyReturn->getRetValue());
 }
 
 // If CMD is one of the forms:
 //   void foo(T arg) { FieldName = arg; }
 //   R foo(T arg) { FieldName = arg; return *this; }
+//   void foo(T arg) { FieldName = std::move(arg); }
+//   R foo(T arg) { FieldName = std::move(arg); return *this; }
 // then returns "FieldName"
 llvm::Optional<StringRef> setterVariableName(const CXXMethodDecl *CMD) {
   assert(CMD->hasBody());
   if (CMD->isConst() || CMD->getNumParams() != 1 || CMD->isVariadic())
     return llvm::None;
   const ParmVarDecl *Arg = CMD->getParamDecl(0);
   if (Arg->isParameterPack())
     return llvm::None;
RHS = BO->getRHS();
                  llvm::dyn_cast<CXXOperatorCallExpr>(Body->body_front())) {
     if (COCE->getOperator() != OO_Equal || COCE->getNumArgs() != 2)
       return llvm::None;
     LHS = COCE->getArg(0);
     RHS = COCE->getArg(1);
   } else {
     return llvm::None;
   }
+
+  // Detect the case when the item is moved into the field.
+  if (auto *CE = llvm::dyn_cast<CallExpr>(RHS->IgnoreCasts())) {
+    if (CE->getNumArgs() != 1)
+      return llvm::None;
+    auto *ND = llvm::dyn_cast<NamedDecl>(CE->getCalleeDecl());
+    if (!ND || !ND->getIdentifier() || ND->getName() != "move" ||
+        !ND->isInStdNamespace())
+      return llvm::None;
+    RHS = CE->getArg(0);
+  }
+
   auto *DRE = llvm::dyn_cast<DeclRefExpr>(RHS->IgnoreCasts());
   if (!DRE || DRE->getDecl() != Arg)
     return llvm::None;
   return fieldName(LHS);
 }
 
 std::string synthesizeDocumentation(const NamedDecl *ND) {
   if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(ND)) {

clang-tools-extra/clangd/unittests/HoverTests.cpp

class Foo {})cpp";
          HI.Type = "struct X &(float)";
          HI.ReturnType = "struct X &";
          HI.Parameters.emplace();
          HI.Parameters->emplace_back();
          HI.Parameters->back().Type = "float";
          HI.Parameters->back().Name = "v";
          HI.AccessSpecifier = "public";
        }},
+      {// Setter (move)
+       R"cpp(
+          namespace std { template<typename T> T&& move(T&& t); }
+          struct X { int Y; void [[^setY]](float v) { Y = std::move(v); } };
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "setY";
+         HI.Kind = index::SymbolKind::InstanceMethod;
+         HI.NamespaceScope = "";
+         HI.Definition = "void setY(float v)";
+         HI.LocalScope = "X::";
+         HI.Documentation = "Trivial setter for `Y`.";
+         HI.Type = "void (float)";
+         HI.ReturnType = "void";
+         HI.Parameters.emplace();
+         HI.Parameters->emplace_back();
+         HI.Parameters->back().Type = "float";
+         HI.Parameters->back().Name = "v";
+         HI.AccessSpecifier = "public";
+       }},
       {// Field type initializer.
        R"cpp(
           struct X { int x = 2; };
           X ^[[x]];
           )cpp",
        [](HoverInfo &HI) {
          HI.Name = "x";
          HI.Kind = index::SymbolKind::Variable;
class Foo {})cpp";
          HI.Name = "yy";
          HI.Kind = index::SymbolKind::Field;
          HI.NamespaceScope = "";
          HI.Definition = "int yy";
          HI.LocalScope = "Foo::";
          HI.Type = "int";
          HI.AccessSpecifier = "public";
        }},
-       {// No crash on InitListExpr.
-        R"cpp(
           struct Foo {
             int a[10];
           };
           constexpr Foo k2 = {
             ^[[{]]1} // FIXME: why the hover range is 1 character?
           };
          )cpp",
        [](HoverInfo &HI) {