adamcz updated this revision to Diff 273742.
adamcz marked 2 inline comments as done.
adamcz added a comment.
Formatting changed to spell out [const] reference and hide type unless
conversion happens.
Defining what the "correct" behavior is on various cases is very difficult.
This is my best shot at incorporating previous requests and comments.
Also addressed more review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81169/new/
https://reviews.llvm.org/D81169
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 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) {
Index: clang-tools-extra/clangd/Hover.h
===================================================================
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -77,11 +77,33 @@
llvm::Optional<uint64_t> Size;
/// Contains the offset of fields within the enclosing class.
llvm::Optional<uint64_t> Offset;
+ // Set when symbol is inside function call. Contains information extracted
+ // from the callee definition about the argument this is passed as.
+ llvm::Optional<Param> 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<PassType> 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);
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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<CallExpr>();
+ 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<Expr>())
+ 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<Expr>()) {
+ 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<ImplicitCastExpr>()) {
+ 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<CXXConstructExpr>()) {
+ // 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<HoverInfo> 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<Expr>()) {
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
