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