Skip to content

Commit 8abf5ae

Browse files
tcottintcottin
committed
[clangd] fix more review findings
1 parent f34993d commit 8abf5ae

File tree

7 files changed

+159
-162
lines changed

7 files changed

+159
-162
lines changed

‎clang-tools-extra/clangd/Hover.cpp‎

Lines changed: 102 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,6 +1393,93 @@ static std::string formatOffset(uint64_t OffsetInBits) {
13931393
return Offset;
13941394
}
13951395

1396+
void HoverInfo::calleeArgInfoToMarkupParagraph(markup::Paragraph &P) const {
1397+
assert(CallPassType);
1398+
std::string Buffer;
1399+
llvm::raw_string_ostream OS(Buffer);
1400+
OS << "Passed ";
1401+
if (CallPassType->PassBy != HoverInfo::PassType::Value) {
1402+
OS << "by ";
1403+
if (CallPassType->PassBy == HoverInfo::PassType::ConstRef)
1404+
OS << "const ";
1405+
OS << "reference ";
1406+
}
1407+
if (CalleeArgInfo->Name)
1408+
OS << "as " << CalleeArgInfo->Name;
1409+
else if (CallPassType->PassBy == HoverInfo::PassType::Value)
1410+
OS << "by value";
1411+
if (CallPassType->Converted && CalleeArgInfo->Type)
1412+
OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
1413+
P.appendText(OS.str());
1414+
}
1415+
1416+
void HoverInfo::usedSymbolNamesToMarkup(markup::Document &Output) const {
1417+
markup::Paragraph &P = Output.addParagraph();
1418+
P.appendText("provides ");
1419+
1420+
const std::vector<std::string>::size_type SymbolNamesLimit = 5;
1421+
auto Front = llvm::ArrayRef(UsedSymbolNames).take_front(SymbolNamesLimit);
1422+
1423+
llvm::interleave(
1424+
Front, [&](llvm::StringRef Sym) { P.appendCode(Sym); },
1425+
[&] { P.appendText(", "); });
1426+
if (UsedSymbolNames.size() > Front.size()) {
1427+
P.appendText(" and ");
1428+
P.appendText(std::to_string(UsedSymbolNames.size() - Front.size()));
1429+
P.appendText(" more");
1430+
}
1431+
}
1432+
1433+
void HoverInfo::providerToMarkupParagraph(markup::Document &Output) const {
1434+
markup::Paragraph &DI = Output.addParagraph();
1435+
DI.appendText("provided by");
1436+
DI.appendSpace();
1437+
DI.appendCode(Provider);
1438+
}
1439+
1440+
void HoverInfo::definitionScopeToMarkup(markup::Document &Output) const {
1441+
std::string Buffer;
1442+
1443+
// Append scope comment, dropping trailing "::".
1444+
// Note that we don't print anything for global namespace, to not annoy
1445+
// non-c++ projects or projects that are not making use of namespaces.
1446+
if (!LocalScope.empty()) {
1447+
// Container name, e.g. class, method, function.
1448+
// We might want to propagate some info about container type to print
1449+
// function foo, class X, method X::bar, etc.
1450+
Buffer += "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
1451+
} else if (NamespaceScope && !NamespaceScope->empty()) {
1452+
Buffer += "// In namespace " +
1453+
llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
1454+
}
1455+
1456+
if (!AccessSpecifier.empty()) {
1457+
Buffer += AccessSpecifier + ": ";
1458+
}
1459+
1460+
Buffer += Definition;
1461+
1462+
Output.addCodeBlock(Buffer, DefinitionLanguage);
1463+
}
1464+
1465+
void HoverInfo::valueToMarkupParagraph(markup::Paragraph &P) const {
1466+
P.appendText("Value = ");
1467+
P.appendCode(*Value);
1468+
}
1469+
1470+
void HoverInfo::offsetToMarkupParagraph(markup::Paragraph &P) const {
1471+
P.appendText("Offset: " + formatOffset(*Offset));
1472+
}
1473+
1474+
void HoverInfo::sizeToMarkupParagraph(markup::Paragraph &P) const {
1475+
P.appendText("Size: " + formatSize(*Size));
1476+
if (Padding && *Padding != 0) {
1477+
P.appendText(llvm::formatv(" (+{0} padding)", formatSize(*Padding)).str());
1478+
}
1479+
if (Align)
1480+
P.appendText(", alignment " + formatSize(*Align));
1481+
}
1482+
13961483
markup::Document HoverInfo::presentDoxygen() const {
13971484
// NOTE: this function is currently almost identical to presentDefault().
13981485
// This is to have a minimal change when introducing the doxygen parser.
@@ -1420,11 +1507,7 @@ markup::Document HoverInfo::presentDoxygen() const {
14201507
Header.appendCode(Name);
14211508

14221509
if (!Provider.empty()) {
1423-
markup::Paragraph &DI = Output.addParagraph();
1424-
DI.appendText("provided by");
1425-
DI.appendSpace();
1426-
DI.appendCode(Provider);
1427-
Output.addRuler();
1510+
providerToMarkupParagraph(Output);
14281511
}
14291512

14301513
// Put a linebreak after header to increase readability.
@@ -1463,91 +1546,31 @@ markup::Document HoverInfo::presentDoxygen() const {
14631546
llvm::to_string(*Type));
14641547

14651548
if (Value) {
1466-
markup::Paragraph &P = Output.addParagraph();
1467-
P.appendText("Value = ");
1468-
P.appendCode(*Value);
1549+
valueToMarkupParagraph(Output.addParagraph());
14691550
}
14701551

14711552
if (Offset)
1472-
Output.addParagraph().appendText("Offset: " + formatOffset(*Offset));
1553+
offsetToMarkupParagraph(Output.addParagraph());
14731554
if (Size) {
1474-
auto &P = Output.addParagraph().appendText("Size: " + formatSize(*Size));
1475-
if (Padding && *Padding != 0) {
1476-
P.appendText(
1477-
llvm::formatv(" (+{0} padding)", formatSize(*Padding)).str());
1478-
}
1479-
if (Align)
1480-
P.appendText(", alignment " + formatSize(*Align));
1555+
sizeToMarkupParagraph(Output.addParagraph());
14811556
}
14821557

14831558
if (CalleeArgInfo) {
1484-
assert(CallPassType);
1485-
std::string Buffer;
1486-
llvm::raw_string_ostream OS(Buffer);
1487-
OS << "Passed ";
1488-
if (CallPassType->PassBy != HoverInfo::PassType::Value) {
1489-
OS << "by ";
1490-
if (CallPassType->PassBy == HoverInfo::PassType::ConstRef)
1491-
OS << "const ";
1492-
OS << "reference ";
1493-
}
1494-
if (CalleeArgInfo->Name)
1495-
OS << "as " << CalleeArgInfo->Name;
1496-
else if (CallPassType->PassBy == HoverInfo::PassType::Value)
1497-
OS << "by value";
1498-
if (CallPassType->Converted && CalleeArgInfo->Type)
1499-
OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
1500-
Output.addParagraph().appendText(OS.str());
1559+
calleeArgInfoToMarkupParagraph(Output.addParagraph());
15011560
}
15021561

15031562
SymbolDoc.docToMarkup(Output);
15041563

15051564
if (!Definition.empty()) {
15061565
Output.addRuler();
1507-
std::string Buffer;
1508-
1509-
if (!Definition.empty()) {
1510-
// Append scope comment, dropping trailing "::".
1511-
// Note that we don't print anything for global namespace, to not annoy
1512-
// non-c++ projects or projects that are not making use of namespaces.
1513-
if (!LocalScope.empty()) {
1514-
// Container name, e.g. class, method, function.
1515-
// We might want to propagate some info about container type to print
1516-
// function foo, class X, method X::bar, etc.
1517-
Buffer +=
1518-
"// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
1519-
} else if (NamespaceScope && !NamespaceScope->empty()) {
1520-
Buffer += "// In namespace " +
1521-
llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
1522-
}
1523-
1524-
if (!AccessSpecifier.empty()) {
1525-
Buffer += AccessSpecifier + ": ";
1526-
}
1527-
1528-
Buffer += Definition;
1529-
}
1530-
1531-
Output.addCodeBlock(Buffer, DefinitionLanguage);
1566+
definitionScopeToMarkup(Output);
15321567
}
15331568

15341569
if (!UsedSymbolNames.empty()) {
15351570
Output.addRuler();
1536-
markup::Paragraph &P = Output.addParagraph();
1537-
P.appendText("provides ");
1538-
1539-
const std::vector<std::string>::size_type SymbolNamesLimit = 5;
1540-
auto Front = llvm::ArrayRef(UsedSymbolNames).take_front(SymbolNamesLimit);
1541-
1542-
llvm::interleave(
1543-
Front, [&](llvm::StringRef Sym) { P.appendCode(Sym); },
1544-
[&] { P.appendText(", "); });
1545-
if (UsedSymbolNames.size() > Front.size()) {
1546-
P.appendText(" and ");
1547-
P.appendText(std::to_string(UsedSymbolNames.size() - Front.size()));
1548-
P.appendText(" more");
1549-
}
1571+
usedSymbolNamesToMarkup(Output);
15501572
}
1573+
15511574
return Output;
15521575
}
15531576

@@ -1572,11 +1595,7 @@ markup::Document HoverInfo::presentDefault() const {
15721595
Header.appendCode(Name);
15731596

15741597
if (!Provider.empty()) {
1575-
markup::Paragraph &DI = Output.addParagraph();
1576-
DI.appendText("provided by");
1577-
DI.appendSpace();
1578-
DI.appendCode(Provider);
1579-
Output.addRuler();
1598+
providerToMarkupParagraph(Output);
15801599
}
15811600

15821601
// Put a linebreak after header to increase readability.
@@ -1607,91 +1626,30 @@ markup::Document HoverInfo::presentDefault() const {
16071626
llvm::to_string(*Type));
16081627

16091628
if (Value) {
1610-
markup::Paragraph &P = Output.addParagraph();
1611-
P.appendText("Value = ");
1612-
P.appendCode(*Value);
1629+
valueToMarkupParagraph(Output.addParagraph());
16131630
}
16141631

16151632
if (Offset)
1616-
Output.addParagraph().appendText("Offset: " + formatOffset(*Offset));
1633+
offsetToMarkupParagraph(Output.addParagraph());
16171634
if (Size) {
1618-
auto &P = Output.addParagraph().appendText("Size: " + formatSize(*Size));
1619-
if (Padding && *Padding != 0) {
1620-
P.appendText(
1621-
llvm::formatv(" (+{0} padding)", formatSize(*Padding)).str());
1622-
}
1623-
if (Align)
1624-
P.appendText(", alignment " + formatSize(*Align));
1635+
sizeToMarkupParagraph(Output.addParagraph());
16251636
}
16261637

16271638
if (CalleeArgInfo) {
1628-
assert(CallPassType);
1629-
std::string Buffer;
1630-
llvm::raw_string_ostream OS(Buffer);
1631-
OS << "Passed ";
1632-
if (CallPassType->PassBy != HoverInfo::PassType::Value) {
1633-
OS << "by ";
1634-
if (CallPassType->PassBy == HoverInfo::PassType::ConstRef)
1635-
OS << "const ";
1636-
OS << "reference ";
1637-
}
1638-
if (CalleeArgInfo->Name)
1639-
OS << "as " << CalleeArgInfo->Name;
1640-
else if (CallPassType->PassBy == HoverInfo::PassType::Value)
1641-
OS << "by value";
1642-
if (CallPassType->Converted && CalleeArgInfo->Type)
1643-
OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
1644-
Output.addParagraph().appendText(OS.str());
1639+
calleeArgInfoToMarkupParagraph(Output.addParagraph());
16451640
}
16461641

16471642
if (!Documentation.empty())
16481643
parseDocumentation(Documentation, Output);
16491644

16501645
if (!Definition.empty()) {
16511646
Output.addRuler();
1652-
std::string Buffer;
1653-
1654-
if (!Definition.empty()) {
1655-
// Append scope comment, dropping trailing "::".
1656-
// Note that we don't print anything for global namespace, to not annoy
1657-
// non-c++ projects or projects that are not making use of namespaces.
1658-
if (!LocalScope.empty()) {
1659-
// Container name, e.g. class, method, function.
1660-
// We might want to propagate some info about container type to print
1661-
// function foo, class X, method X::bar, etc.
1662-
Buffer +=
1663-
"// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
1664-
} else if (NamespaceScope && !NamespaceScope->empty()) {
1665-
Buffer += "// In namespace " +
1666-
llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
1667-
}
1668-
1669-
if (!AccessSpecifier.empty()) {
1670-
Buffer += AccessSpecifier + ": ";
1671-
}
1672-
1673-
Buffer += Definition;
1674-
}
1675-
1676-
Output.addCodeBlock(Buffer, DefinitionLanguage);
1647+
definitionScopeToMarkup(Output);
16771648
}
16781649

16791650
if (!UsedSymbolNames.empty()) {
16801651
Output.addRuler();
1681-
markup::Paragraph &P = Output.addParagraph();
1682-
P.appendText("provides ");
1683-
1684-
const std::vector<std::string>::size_type SymbolNamesLimit = 5;
1685-
auto Front = llvm::ArrayRef(UsedSymbolNames).take_front(SymbolNamesLimit);
1686-
1687-
llvm::interleave(
1688-
Front, [&](llvm::StringRef Sym) { P.appendCode(Sym); },
1689-
[&] { P.appendText(", "); });
1690-
if (UsedSymbolNames.size() > Front.size()) {
1691-
P.appendText(" and ");
1692-
P.appendText(std::to_string(UsedSymbolNames.size() - Front.size()));
1693-
P.appendText(" more");
1694-
}
1652+
usedSymbolNamesToMarkup(Output);
16951653
}
16961654

16971655
return Output;

‎clang-tools-extra/clangd/Hover.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ struct HoverInfo {
124124
std::string present(MarkupKind Kind) const;
125125

126126
private:
127+
void usedSymbolNamesToMarkup(markup::Document &Output) const;
128+
void providerToMarkupParagraph(markup::Document &Output) const;
129+
void definitionScopeToMarkup(markup::Document &Output) const;
130+
void calleeArgInfoToMarkupParagraph(markup::Paragraph &P) const;
131+
void valueToMarkupParagraph(markup::Paragraph &P) const;
132+
void offsetToMarkupParagraph(markup::Paragraph &P) const;
133+
void sizeToMarkupParagraph(markup::Paragraph &P) const;
134+
127135
/// Parse and render the hover information as Doxygen documentation.
128136
markup::Document presentDoxygen() const;
129137

0 commit comments

Comments
 (0)