v1nh1shungry updated this revision to Diff 491594.
v1nh1shungry added a comment.
land review's suggestions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142014/new/
https://reviews.llvm.org/D142014
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
@@ -934,6 +934,23 @@
HI.CalleeArgInfo->Type = "const int &";
HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
}},
+ {
+ R"cpp(
+ int add(int lhs, int rhs);
+ int main() {
+ add(1 [[^+]] 2, 3);
+ }
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "expression";
+ HI.Kind = index::SymbolKind::Unknown;
+ HI.Type = "int";
+ HI.Value = "3";
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Name = "lhs";
+ HI.CalleeArgInfo->Type = "int";
+ HI.CallPassType = HoverInfo::PassType{PassMode::Value, false};
+ }},
{// Extra info for method call.
R"cpp(
class C {
@@ -1673,8 +1690,8 @@
}},
{
R"cpp(// Function definition via using declaration
- namespace ns {
- void foo();
+ namespace ns {
+ void foo();
}
int main() {
using ns::foo;
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -952,6 +952,15 @@
}
}
+HoverInfo::PassType::PassMode getPassMode(QualType ParmType) {
+ if (ParmType->isReferenceType()) {
+ if (ParmType->getPointeeType().isConstQualified())
+ return HoverInfo::PassType::ConstRef;
+ return HoverInfo::PassType::Ref;
+ }
+ return HoverInfo::PassType::Value;
+}
+
// 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,
@@ -972,14 +981,18 @@
if (!FD || FD->isOverloadedOperator() || FD->isVariadic())
return;
+ HoverInfo::PassType PassType;
+
// Find argument index for N.
for (unsigned I = 0; I < CE->getNumArgs() && I < FD->getNumParams(); ++I) {
if (CE->getArg(I) != OuterNode.ASTNode.get<Expr>())
continue;
// Extract matching argument from function declaration.
- if (const ParmVarDecl *PVD = FD->getParamDecl(I))
+ if (const ParmVarDecl *PVD = FD->getParamDecl(I)) {
HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP));
+ PassType.PassBy = getPassMode(PVD->getType());
+ }
break;
}
if (!HI.CalleeArgInfo)
@@ -988,16 +1001,6 @@
// 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<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;
CastNode != OuterNode.Parent && !PassType.Converted;
CastNode = CastNode->Parent) {
@@ -1006,22 +1009,13 @@
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;
}
@@ -1029,16 +1023,24 @@
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())
+ const CXXConstructorDecl *CD = CtorCall->getConstructor();
+ if (CD->isCopyConstructor()) {
PassType.PassBy = HoverInfo::PassType::Value;
- else
+ } else {
+ if (CD->getNumParams() >= 1)
+ PassType.PassBy = getPassMode(CD->getParamDecl(0)->getType());
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;
+ // We don't want to show this as type conversion, e.g.
+ // void foobar(const int &);
+ // int main() {
+ // foobar(0);
+ // ^ useless to show "converted to const int &" here
+ // }
+ PassType.Converted = false;
} else { // Unknown implicit node, assume type conversion.
- PassType.PassBy = HoverInfo::PassType::Value;
PassType.Converted = true;
}
}
@@ -1068,9 +1070,8 @@
// template <typename T> void bar() { fo^o(T{}); }
// we actually want to show the using declaration,
// it's not clear which declaration to pick otherwise.
- auto BaseDecls = llvm::make_filter_range(Candidates, [](const NamedDecl *D) {
- return llvm::isa<UsingDecl>(D);
- });
+ auto BaseDecls = llvm::make_filter_range(
+ Candidates, [](const NamedDecl *D) { return llvm::isa<UsingDecl>(D); });
if (std::distance(BaseDecls.begin(), BaseDecls.end()) == 1)
return *BaseDecls.begin();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
