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,33 @@ 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 { + // True if variable is passed by reference, either directly or to an + // implicit constructor call. + bool Reference = true; + // True if that reference is false. Never set for non-reference pass. + bool Const = false; + // 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.Reference, LHS.Const, LHS.Converted) == + std::tie(RHS.Reference, RHS.Const, 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,30 +289,35 @@ : PVD->getDefaultArg(); } +HoverInfo::Param toHoverInfoParam(const ParmVarDecl *PVD, + const PrintingPolicy &Policy) { + HoverInfo::Param Out; + if (!PVD->getType().isNull()) { + Out.Type = printType(PVD->getType(), Policy); + } else { + std::string Param; + llvm::raw_string_ostream OS(Param); + PVD->dump(OS); + OS.flush(); + elog("Got param with null type: {0}", Param); + } + 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(); - if (!PVD->getType().isNull()) { - P.Type = printType(PVD->getType(), Policy); - } else { - std::string Param; - llvm::raw_string_ostream OS(Param); - PVD->dump(OS); - OS.flush(); - elog("Got param with null type: {0}", Param); - } - 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); - } + HI.Parameters->emplace_back(toHoverInfoParam(PVD, Policy)); } // We don't want any type info, if name already contains it. This is true for @@ -686,6 +691,97 @@ } } +// 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; + 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) { + auto *Arg = CE->getArg(I); + if (Arg != 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.hasValue()) + 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 (auto *E = N->ASTNode.get()) { + if (E->getType().isConstQualified()) { + PassType.Const = true; + } + } + + for (auto *CastNode = N->Parent; + CastNode != OuterNode.Parent && !PassType.Converted; + CastNode = CastNode->Parent) { + if (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.Reference) + PassType.Const = ImplicitCast->getType().isConstQualified(); + 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.Reference = false; + PassType.Const = false; + break; + default: + PassType.Reference = false; + PassType.Const = false; + PassType.Converted = true; + break; + } + } else if (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.Reference = false; + PassType.Const = false; + } else { + PassType.Converted = true; + } + } else { // Unknown implicit node, assume type conversion. + PassType.Reference = false; + PassType.Const = false; + PassType.Converted = true; + } + } + + HI.CallPassType.emplace(PassType); +} + } // namespace llvm::Optional getHover(ParsedAST &AST, Position Pos, @@ -748,6 +844,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); } @@ -828,6 +925,23 @@ 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->Reference) { + OS << "by " << (CallPassType->Const ? "const " : "") << "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); @@ -852,6 +966,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 @@ -708,6 +708,59 @@ HI.Definition = "X x"; HI.Type = "struct X"; }}, + {// 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->Reference = true; + HI.CallPassType->Const = false; + 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->Reference = false; + HI.CallPassType->Const = false; + HI.CallPassType->Converted = false; + }}, }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Code); @@ -741,6 +794,87 @@ 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; + bool Reference; + bool Const; + bool Converted; + } Tests[] = { + // Integer tests + {"int_by_value([[^int_x]]);", false, false, false}, + {"int_by_ref([[^int_x]]);", true, false, false}, + {"int_by_const_ref([[^int_x]]);", true, true, false}, + {"int_by_value([[^int_ref]]);", false, false, false}, + {"int_by_const_ref([[^int_ref]]);", true, true, false}, + {"int_by_const_ref([[^int_ref]]);", true, true, false}, + {"int_by_const_ref([[^int_const_ref]]);", true, true, false}, + // Custom class tests + {"base_by_ref([[^base]]);", true, false, false}, + {"base_by_const_ref([[^base]]);", true, true, false}, + {"base_by_const_ref([[^base_const_ref]]);", true, true, false}, + {"base_by_value([[^base]]);", false, false, false}, + {"base_by_value([[^base_const_ref]]);", false, false, false}, + {"base_by_ref([[^derived]]);", true, false, false}, + {"base_by_const_ref([[^derived]]);", true, true, false}, + {"base_by_value([[^derived]]);", false, false, false}, + // Converted tests + {"float_by_value([[^int_x]]);", false, false, true}, + {"float_by_value([[^int_ref]]);", false, false, true}, + {"float_by_value([[^int_const_ref]]);", false, false, true}, + {"custom_by_value([[^int_x]]);", true, false, true}, + {"custom_by_value([[^float_x]]);", false, false, true}, + {"custom_by_value([[^base]]);", true, true, 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->Reference, Test.Reference); + EXPECT_EQ(H->CallPassType->Const, Test.Const); + EXPECT_EQ(H->CallPassType->Converted, Test.Converted); } } @@ -2032,6 +2166,110 @@ // 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->Reference = false; + HI.CallPassType->Const = false; + 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->Reference = true; + HI.CallPassType->Const = false; + 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->Reference = false; + HI.CallPassType->Const = false; + 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->Reference = true; + HI.CallPassType->Const = true; + 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) {