[clang-tools-extra] 343b1ae - [clangd] Hover: show CalleeArgInfo for literals and expressions
Tom Praschan via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 14 13:21:30 PST 2023
Author: Tom Praschan
Date: 2023-01-14T23:20:42+01:00
New Revision: 343b1ae3622afed50c9734a8f40b368d8afef5c5
URL: https://github.com/llvm/llvm-project/commit/343b1ae3622afed50c9734a8f40b368d8afef5c5
DIFF: https://github.com/llvm/llvm-project/commit/343b1ae3622afed50c9734a8f40b368d8afef5c5.diff
LOG: [clangd] Hover: show CalleeArgInfo for literals and expressions
This is very useful when inlay hints are disabled.
Also, improve presentation of Hover when variable is passed by value to
a function with an unnamed parameter
Differential Revision: https://reviews.llvm.org/D140775
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index d05ea6a10f6f..7f09e091afcb 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -795,7 +795,7 @@ bool isLiteral(const Expr *E) {
llvm::isa<CXXNullPtrLiteralExpr>(E) ||
llvm::isa<FixedPointLiteral>(E) || llvm::isa<FloatingLiteral>(E) ||
llvm::isa<ImaginaryLiteral>(E) || llvm::isa<IntegerLiteral>(E) ||
- llvm::isa<UserDefinedLiteral>(E);
+ llvm::isa<StringLiteral>(E) || llvm::isa<UserDefinedLiteral>(E);
}
llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -809,34 +809,53 @@ llvm::StringLiteral getNameForExpr(const Expr *E) {
return llvm::StringLiteral("expression");
}
+void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
+ const PrintingPolicy &PP);
+
// Generates hover info for `this` and evaluatable expressions.
// FIXME: Support hover for literals (esp user-defined)
-std::optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
+std::optional<HoverInfo> getHoverContents(const SelectionTree::Node *N,
+ const Expr *E, ParsedAST &AST,
const PrintingPolicy &PP,
const SymbolIndex *Index) {
- // There's not much value in hovering over "42" and getting a hover card
- // saying "42 is an int", similar for other literals.
- if (isLiteral(E))
+ std::optional<HoverInfo> HI;
+
+ if (const StringLiteral *SL = dyn_cast<StringLiteral>(E)) {
+ // Print the type and the size for string literals
+ HI = getStringLiteralContents(SL, PP);
+ } else if (isLiteral(E)) {
+ // There's not much value in hovering over "42" and getting a hover card
+ // saying "42 is an int", similar for most other literals.
+ // However, if we have CalleeArgInfo, it's still useful to show it.
+ maybeAddCalleeArgInfo(N, HI.emplace(), PP);
+ if (HI->CalleeArgInfo) {
+ // FIXME Might want to show the expression's value here instead?
+ // E.g. if the literal is in hex it might be useful to show the decimal
+ // value here.
+ HI->Name = "literal";
+ return HI;
+ }
return std::nullopt;
+ }
- HoverInfo HI;
- // Print the type and the size for string literals
- if (const StringLiteral *SL = dyn_cast<StringLiteral>(E))
- return getStringLiteralContents(SL, PP);
// For `this` expr we currently generate hover with pointee type.
if (const CXXThisExpr *CTE = dyn_cast<CXXThisExpr>(E))
- return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
+ HI = getThisExprHoverContents(CTE, AST.getASTContext(), PP);
if (const PredefinedExpr *PE = dyn_cast<PredefinedExpr>(E))
- return getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
+ HI = getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
// For expressions we currently print the type and the value, iff it is
// evaluatable.
if (auto Val = printExprValue(E, AST.getASTContext())) {
- HI.Type = printType(E->getType(), AST.getASTContext(), PP);
- HI.Value = *Val;
- HI.Name = std::string(getNameForExpr(E));
- return HI;
+ HI.emplace();
+ HI->Type = printType(E->getType(), AST.getASTContext(), PP);
+ HI->Value = *Val;
+ HI->Name = std::string(getNameForExpr(E));
}
- return std::nullopt;
+
+ if (HI)
+ maybeAddCalleeArgInfo(N, *HI, PP);
+
+ return HI;
}
// Generates hover info for attributes.
@@ -973,6 +992,10 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
if (const auto *E = N->ASTNode.get<Expr>()) {
if (E->getType().isConstQualified())
PassType.PassBy = HoverInfo::PassType::ConstRef;
+
+ // No implicit node, literal passed by value
+ if (isLiteral(E) && N->Parent == OuterNode.Parent)
+ PassType.PassBy = HoverInfo::PassType::Value;
}
for (auto *CastNode = N->Parent;
@@ -1010,6 +1033,10 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
PassType.PassBy = HoverInfo::PassType::Value;
else
PassType.Converted = true;
+ } else if (const auto *MTE =
+ CastNode->ASTNode.get<MaterializeTemporaryExpr>()) {
+ // Can't bind a non-const-ref to a temporary, so has to be const-ref
+ PassType.PassBy = HoverInfo::PassType::ConstRef;
} else { // Unknown implicit node, assume type conversion.
PassType.PassBy = HoverInfo::PassType::Value;
PassType.Converted = true;
@@ -1135,7 +1162,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
HI->Value = printExprValue(N, AST.getASTContext());
maybeAddCalleeArgInfo(N, *HI, PP);
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
- HI = getHoverContents(E, AST, PP, Index);
+ HI = getHoverContents(N, E, AST, PP, Index);
} else if (const Attr *A = N->ASTNode.get<Attr>()) {
HI = getHoverContents(A, AST);
}
@@ -1240,6 +1267,8 @@ markup::Document HoverInfo::present() const {
}
if (CalleeArgInfo->Name)
OS << "as " << CalleeArgInfo->Name;
+ else if (CallPassType->PassBy == HoverInfo::PassType::Value)
+ OS << "by value";
if (CallPassType->Converted && CalleeArgInfo->Type)
OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
Output.addParagraph().appendText(OS.str());
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 3493813069c4..7fbb8858c3f4 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -900,6 +900,40 @@ class Foo final {})cpp";
HI.CalleeArgInfo->Type = "int &";
HI.CallPassType = HoverInfo::PassType{PassMode::Ref, false};
}},
+ {// Literal passed to function call
+ R"cpp(
+ void fun(int arg_a, const int &arg_b) {};
+ void code() {
+ int a = 1;
+ fun(a, [[^2]]);
+ }
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "literal";
+ HI.Kind = index::SymbolKind::Unknown;
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Name = "arg_b";
+ HI.CalleeArgInfo->Type = "const int &";
+ HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+ }},
+ {// Expression passed to function call
+ R"cpp(
+ void fun(int arg_a, const int &arg_b) {};
+ void code() {
+ int a = 1;
+ fun(a, 1 [[^+]] 2);
+ }
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "expression";
+ HI.Kind = index::SymbolKind::Unknown;
+ HI.Type = "int";
+ HI.Value = "3";
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Name = "arg_b";
+ HI.CalleeArgInfo->Type = "const int &";
+ HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+ }},
{// Extra info for method call.
R"cpp(
class C {
@@ -1226,8 +1260,10 @@ void fun() {
} Tests[] = {
// Integer tests
{"int_by_value([[^int_x]]);", PassMode::Value, false},
+ {"int_by_value([[^123]]);", PassMode::Value, false},
{"int_by_ref([[^int_x]]);", PassMode::Ref, false},
{"int_by_const_ref([[^int_x]]);", PassMode::ConstRef, false},
+ {"int_by_const_ref([[^123]]);", 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},
@@ -1245,6 +1281,8 @@ void fun() {
{"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},
+ {"float_by_value([[^123.0f]]);", PassMode::Value, false},
+ {"float_by_value([[^123]]);", 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},
@@ -3043,6 +3081,18 @@ Passed as arg_a
// In test::Bar
int foo = 3)",
+ },
+ {
+ [](HoverInfo &HI) {
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Name = "foo";
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Type = "int";
+ HI.CallPassType = HoverInfo::PassType{PassMode::Value, false};
+ },
+ R"(variable foo
+
+Passed by value)",
},
{
[](HoverInfo &HI) {
More information about the cfe-commits
mailing list