tom-anders updated this revision to Diff 485761.
tom-anders marked 4 inline comments as done.
tom-anders added a comment.
Refactor getHoverContents to add CalleeArgInfo for all kinds of expression
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140775/new/
https://reviews.llvm.org/D140775
Files:
clang-tools-extra/clangd/Hover.cpp
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
@@ -900,6 +900,22 @@
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};
+ }},
{// Extra info for method call.
R"cpp(
class C {
@@ -1226,8 +1242,10 @@
} 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 +1263,8 @@
{"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},
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -795,7 +795,7 @@
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 @@
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 @@
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 @@
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 @@
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);
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
