diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h --- a/clang-tools-extra/clangd/Hover.h +++ b/clang-tools-extra/clangd/Hover.h @@ -77,11 +77,31 @@ llvm::Optional Size; /// Contains the offset of fields within the enclosing class. llvm::Optional Offset; + // Set when symbol is inside function call. Contains information extracted + // from the callee definition about the argument this is passed as. + llvm::Optional CalleeArgInfo; + struct PassType { + // How the variable is passed to callee. + enum PassMode { Ref, ConstRef, Value }; + PassMode PassBy = Ref; + // True if type conversion happened. This includes calls to implicit + // constructor, as well as built-in type conversions. Casting to base class + // is not considered conversion. + bool Converted = false; + }; + // Set only if CalleeArgInfo is set. + llvm::Optional CallPassType; /// Produce a user-readable information. markup::Document present() const; }; +inline bool operator==(const HoverInfo::PassType &LHS, + const HoverInfo::PassType &RHS) { + return std::tie(LHS.PassBy, LHS.Converted) == + std::tie(RHS.PassBy, RHS.Converted); +} + // Try to infer structure of a documentation comment (e.g. line breaks). // FIXME: move to another file so CodeComplete doesn't depend on Hover. void parseDocumentation(llvm::StringRef Input, markup::Document &Output); diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -289,23 +289,27 @@ : PVD->getDefaultArg(); } +HoverInfo::Param toHoverInfoParam(const ParmVarDecl *PVD, + const PrintingPolicy &Policy) { + HoverInfo::Param Out; + Out.Type = printType(PVD->getType(), Policy); + if (!PVD->getName().empty()) + Out.Name = PVD->getNameAsString(); + if (const Expr *DefArg = getDefaultArg(PVD)) { + Out.Default.emplace(); + llvm::raw_string_ostream OS(*Out.Default); + DefArg->printPretty(OS, nullptr, Policy); + } + return Out; +} + // Populates Type, ReturnType, and Parameters for function-like decls. void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D, const FunctionDecl *FD, const PrintingPolicy &Policy) { HI.Parameters.emplace(); - for (const ParmVarDecl *PVD : FD->parameters()) { - HI.Parameters->emplace_back(); - auto &P = HI.Parameters->back(); - P.Type = printType(PVD->getType(), Policy); - if (!PVD->getName().empty()) - P.Name = PVD->getNameAsString(); - if (const Expr *DefArg = getDefaultArg(PVD)) { - P.Default.emplace(); - llvm::raw_string_ostream Out(*P.Default); - DefArg->printPretty(Out, nullptr, Policy); - } - } + for (const ParmVarDecl *PVD : FD->parameters()) + HI.Parameters->emplace_back(toHoverInfoParam(PVD, Policy)); // We don't want any type info, if name already contains it. This is true for // constructors/destructors and conversion operators. @@ -678,6 +682,92 @@ } } +// If N is passed as argument to a function, fill HI.CalleeArgInfo with +// information about that argument. +void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI, + const PrintingPolicy &Policy) { + const auto &OuterNode = N->outerImplicit(); + if (!OuterNode.Parent) + return; + const auto *CE = OuterNode.Parent->ASTNode.get(); + if (!CE) + return; + const FunctionDecl *FD = CE->getDirectCallee(); + // For non-function-call-like operatators (e.g. operator+, operator<<) it's + // not immediattely obvious what the "passed as" would refer to and, given + // fixed function signature, the value would be very low anyway, so we choose + // to not support that. + // Both variadic functions and operator() (especially relevant for lambdas) + // should be supported in the future. + if (!FD || FD->isOverloadedOperator() || FD->isVariadic()) + return; + + // Find argument index for N. + for (unsigned I = 0; I < CE->getNumArgs() && I < FD->getNumParams(); ++I) { + if (CE->getArg(I) != OuterNode.ASTNode.get()) + continue; + + // Extract matching argument from function declaration. + if (const ParmVarDecl *PVD = FD->getParamDecl(I)) + HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, Policy)); + break; + } + if (!HI.CalleeArgInfo) + return; + + // If we found a matching argument, also figure out if it's a + // [const-]reference. For this we need to walk up the AST from the arg itself + // to CallExpr and check all implicit casts, constructor calls, etc. + HoverInfo::PassType PassType; + if (const auto *E = N->ASTNode.get()) { + if (E->getType().isConstQualified()) + PassType.PassBy = HoverInfo::PassType::ConstRef; + } + + for (auto *CastNode = N->Parent; + CastNode != OuterNode.Parent && !PassType.Converted; + CastNode = CastNode->Parent) { + if (const auto *ImplicitCast = CastNode->ASTNode.get()) { + switch (ImplicitCast->getCastKind()) { + case CK_NoOp: + case CK_DerivedToBase: + case CK_UncheckedDerivedToBase: + // If it was a reference before, it's still a reference. + if (PassType.PassBy != HoverInfo::PassType::Value) + PassType.PassBy = ImplicitCast->getType().isConstQualified() + ? HoverInfo::PassType::ConstRef + : HoverInfo::PassType::Ref; + break; + case CK_LValueToRValue: + case CK_ArrayToPointerDecay: + case CK_FunctionToPointerDecay: + case CK_NullToPointer: + case CK_NullToMemberPointer: + // No longer a reference, but we do not show this as type conversion. + PassType.PassBy = HoverInfo::PassType::Value; + break; + default: + PassType.PassBy = HoverInfo::PassType::Value; + PassType.Converted = true; + break; + } + } else if (const auto *CtorCall = + CastNode->ASTNode.get()) { + // We want to be smart about copy constructors. They should not show up as + // type conversion, but instead as passing by value. + if (CtorCall->getConstructor()->isCopyConstructor()) + PassType.PassBy = HoverInfo::PassType::Value; + else + PassType.Converted = true; + } else { // Unknown implicit node, assume type conversion. + PassType.PassBy = HoverInfo::PassType::Value; + PassType.Converted = true; + } + } + + HI.CallPassType.emplace(PassType); +} + } // namespace llvm::Optional getHover(ParsedAST &AST, Position Pos, @@ -740,6 +830,7 @@ // Look for a close enclosing expression to show the value of. if (!HI->Value) HI->Value = printExprValue(N, AST.getASTContext()); + maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy()); } else if (const Expr *E = N->ASTNode.get()) { HI = getHoverContents(E, AST); } @@ -820,6 +911,24 @@ Output.addParagraph().appendText( llvm::formatv("Size: {0} byte{1}", *Size, *Size == 1 ? "" : "s").str()); + if (CalleeArgInfo) { + assert(CallPassType); + std::string Buffer; + llvm::raw_string_ostream OS(Buffer); + OS << "Passed "; + if (CallPassType->PassBy != HoverInfo::PassType::Value) { + OS << "by "; + if (CallPassType->PassBy == HoverInfo::PassType::ConstRef) + OS << "const "; + OS << "reference "; + } + if (CalleeArgInfo->Name) + OS << "as " << CalleeArgInfo->Name; + if (CallPassType->Converted && CalleeArgInfo->Type) + OS << " (converted to " << CalleeArgInfo->Type << ")"; + Output.addParagraph().appendText(OS.str()); + } + if (!Documentation.empty()) parseDocumentation(Documentation, Output); @@ -844,6 +953,7 @@ // non-c++ projects or projects that are not making use of namespaces. Output.addCodeBlock(ScopeComment + DefinitionWithAccess); } + return Output; } diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -26,6 +26,8 @@ namespace clangd { namespace { +using PassMode = HoverInfo::PassType::PassMode; + TEST(Hover, Structured) { struct { const char *const Code; @@ -719,6 +721,57 @@ // Bindings are in theory public members of an anonymous struct. HI.AccessSpecifier = "public"; }}, + {// Extra info for function call. + R"cpp( + void fun(int arg_a, int &arg_b) {}; + void code() { + int a = 1, b = 2; + fun(a, [[^b]]); + } + )cpp", + [](HoverInfo &HI) { + HI.Name = "b"; + HI.Kind = index::SymbolKind::Variable; + HI.NamespaceScope = ""; + HI.Definition = "int b = 2"; + HI.LocalScope = "code::"; + HI.Value = "2"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_b"; + HI.CalleeArgInfo->Type = "int &"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::Ref; + HI.CallPassType->Converted = false; + }}, + {// Extra info for method call. + R"cpp( + class C { + public: + void fun(int arg_a = 3, int arg_b = 4) {} + }; + void code() { + int a = 1, b = 2; + C c; + c.fun([[^a]], b); + } + )cpp", + [](HoverInfo &HI) { + HI.Name = "a"; + HI.Kind = index::SymbolKind::Variable; + HI.NamespaceScope = ""; + HI.Definition = "int a = 1"; + HI.LocalScope = "code::"; + HI.Value = "1"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_a"; + HI.CalleeArgInfo->Type = "int"; + HI.CalleeArgInfo->Default = "3"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::Value; + HI.CallPassType->Converted = false; + }}, }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Code); @@ -752,6 +805,85 @@ EXPECT_EQ(H->Size, Expected.Size); EXPECT_EQ(H->Offset, Expected.Offset); EXPECT_EQ(H->AccessSpecifier, Expected.AccessSpecifier); + EXPECT_EQ(H->CalleeArgInfo, Expected.CalleeArgInfo); + EXPECT_EQ(H->CallPassType, Expected.CallPassType); + } +} + +TEST(Hover, CallPassType) { + const llvm::StringRef CodePrefix = R"cpp( +class Base {}; +class Derived : public Base {}; +class CustomClass { + public: + CustomClass() {} + CustomClass(const Base &x) {} + CustomClass(int &x) {} + CustomClass(float x) {} +}; + +void int_by_ref(int &x) {} +void int_by_const_ref(const int &x) {} +void int_by_value(int x) {} +void base_by_ref(Base &x) {} +void base_by_const_ref(const Base &x) {} +void base_by_value(Base x) {} +void float_by_value(float x) {} +void custom_by_value(CustomClass x) {} + +void fun() { + int int_x; + int &int_ref = int_x; + const int &int_const_ref = int_x; + Base base; + const Base &base_const_ref = base; + Derived derived; + float float_x; +)cpp"; + const llvm::StringRef CodeSuffix = "}"; + + struct { + const char *const Code; + HoverInfo::PassType::PassMode PassBy; + bool Converted; + } Tests[] = { + // Integer tests + {"int_by_value([[^int_x]]);", PassMode::Value, false}, + {"int_by_ref([[^int_x]]);", PassMode::Ref, false}, + {"int_by_const_ref([[^int_x]]);", PassMode::ConstRef, false}, + {"int_by_value([[^int_ref]]);", PassMode::Value, false}, + {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false}, + {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false}, + {"int_by_const_ref([[^int_const_ref]]);", PassMode::ConstRef, false}, + // Custom class tests + {"base_by_ref([[^base]]);", PassMode::Ref, false}, + {"base_by_const_ref([[^base]]);", PassMode::ConstRef, false}, + {"base_by_const_ref([[^base_const_ref]]);", PassMode::ConstRef, false}, + {"base_by_value([[^base]]);", PassMode::Value, false}, + {"base_by_value([[^base_const_ref]]);", PassMode::Value, false}, + {"base_by_ref([[^derived]]);", PassMode::Ref, false}, + {"base_by_const_ref([[^derived]]);", PassMode::ConstRef, false}, + {"base_by_value([[^derived]]);", PassMode::Value, false}, + // Converted tests + {"float_by_value([[^int_x]]);", PassMode::Value, true}, + {"float_by_value([[^int_ref]]);", PassMode::Value, true}, + {"float_by_value([[^int_const_ref]]);", PassMode::Value, true}, + {"custom_by_value([[^int_x]]);", PassMode::Ref, true}, + {"custom_by_value([[^float_x]]);", PassMode::Value, true}, + {"custom_by_value([[^base]]);", PassMode::ConstRef, true}, + }; + for (const auto &Test : Tests) { + SCOPED_TRACE(Test.Code); + + const auto Code = (CodePrefix + Test.Code + CodeSuffix).str(); + Annotations T(Code); + TestTU TU = TestTU::withCode(T.code()); + TU.ExtraArgs.push_back("-std=c++17"); + auto AST = TU.build(); + auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr); + ASSERT_TRUE(H); + EXPECT_EQ(H->CallPassType->PassBy, Test.PassBy); + EXPECT_EQ(H->CallPassType->Converted, Test.Converted); } } @@ -2043,6 +2175,106 @@ // In namespace ns1 private: union foo {})", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Variable; + HI.Name = "foo"; + HI.Definition = "int foo = 3"; + HI.LocalScope = "test::Bar::"; + HI.Value = "3"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_a"; + HI.CalleeArgInfo->Type = "int"; + HI.CalleeArgInfo->Default = "7"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::Value; + HI.CallPassType->Converted = false; + }, + R"(variable foo + +Type: int +Value = 3 +Passed as arg_a + +// In test::Bar +int foo = 3)", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Variable; + HI.Name = "foo"; + HI.Definition = "int foo = 3"; + HI.LocalScope = "test::Bar::"; + HI.Value = "3"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_a"; + HI.CalleeArgInfo->Type = "int"; + HI.CalleeArgInfo->Default = "7"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::Ref; + HI.CallPassType->Converted = false; + }, + R"(variable foo + +Type: int +Value = 3 +Passed by reference as arg_a + +// In test::Bar +int foo = 3)", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Variable; + HI.Name = "foo"; + HI.Definition = "int foo = 3"; + HI.LocalScope = "test::Bar::"; + HI.Value = "3"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_a"; + HI.CalleeArgInfo->Type = "int"; + HI.CalleeArgInfo->Default = "7"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::Value; + HI.CallPassType->Converted = true; + }, + R"(variable foo + +Type: int +Value = 3 +Passed as arg_a (converted to int) + +// In test::Bar +int foo = 3)", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Variable; + HI.Name = "foo"; + HI.Definition = "int foo = 3"; + HI.LocalScope = "test::Bar::"; + HI.Value = "3"; + HI.Type = "int"; + HI.CalleeArgInfo.emplace(); + HI.CalleeArgInfo->Name = "arg_a"; + HI.CalleeArgInfo->Type = "int"; + HI.CalleeArgInfo->Default = "7"; + HI.CallPassType.emplace(); + HI.CallPassType->PassBy = PassMode::ConstRef; + HI.CallPassType->Converted = true; + }, + R"(variable foo + +Type: int +Value = 3 +Passed by const reference as arg_a (converted to int) + +// In test::Bar +int foo = 3)", }}; for (const auto &C : Cases) {