[clang-tools-extra] ee032bc - [clangd] Add BlockEnd comments for control flow statements
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 21 13:57:53 PDT 2023
Author: Sam McCall
Date: 2023-07-21T22:57:46+02:00
New Revision: ee032bccc934d034909962a9306f7c9d1ca72759
URL: https://github.com/llvm/llvm-project/commit/ee032bccc934d034909962a9306f7c9d1ca72759
DIFF: https://github.com/llvm/llvm-project/commit/ee032bccc934d034909962a9306f7c9d1ca72759.diff
LOG: [clangd] Add BlockEnd comments for control flow statements
These mark the end of CompoundStmts bodies of if/while/for/switch.
To identify which statement is being ended, we include abbreviated
text of the condition/loop variable.
Differential Revision: https://reviews.llvm.org/D155421
Added:
Modified:
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 1347fa0d2b29e0..0b5bab1aac9b1b 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -14,13 +14,26 @@
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclarationName.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
#include <optional>
+#include <string>
namespace clang {
namespace clangd {
@@ -192,6 +205,202 @@ getDesignators(const InitListExpr *Syn) {
return Designators;
}
+void stripLeadingUnderscores(StringRef &Name) { Name = Name.ltrim('_'); }
+
+// getDeclForType() returns the decl responsible for Type's spelling.
+// This is the inverse of ASTContext::getTypeDeclType().
+template <typename Ty, typename = decltype(((Ty *)nullptr)->getDecl())>
+const NamedDecl *getDeclForTypeImpl(const Ty *T) {
+ return T->getDecl();
+}
+const NamedDecl *getDeclForTypeImpl(const void *T) { return nullptr; }
+const NamedDecl *getDeclForType(const Type *T) {
+ switch (T->getTypeClass()) {
+#define ABSTRACT_TYPE(TY, BASE)
+#define TYPE(TY, BASE) \
+ case Type::TY: \
+ return getDeclForTypeImpl(llvm::cast<TY##Type>(T));
+#include "clang/AST/TypeNodes.inc"
+ }
+}
+
+// getSimpleName() returns the plain identifier for an entity, if any.
+llvm::StringRef getSimpleName(const DeclarationName &DN) {
+ if (IdentifierInfo *Ident = DN.getAsIdentifierInfo())
+ return Ident->getName();
+ return "";
+}
+llvm::StringRef getSimpleName(const NamedDecl &D) {
+ return getSimpleName(D.getDeclName());
+}
+llvm::StringRef getSimpleName(QualType T) {
+ if (const auto *ET = llvm::dyn_cast<ElaboratedType>(T))
+ return getSimpleName(ET->getNamedType());
+ if (const auto *BT = llvm::dyn_cast<BuiltinType>(T)) {
+ PrintingPolicy PP(LangOptions{});
+ PP.adjustForCPlusPlus();
+ return BT->getName(PP);
+ }
+ if (const auto *D = getDeclForType(T.getTypePtr()))
+ return getSimpleName(D->getDeclName());
+ return "";
+}
+
+// Returns a very abbreviated form of an expression, or "" if it's too complex.
+// For example: `foo->bar()` would produce "bar".
+// This is used to summarize e.g. the condition of a while loop.
+std::string summarizeExpr(const Expr *E) {
+ struct Namer : ConstStmtVisitor<Namer, std::string> {
+ std::string Visit(const Expr *E) {
+ if (E == nullptr)
+ return "";
+ return ConstStmtVisitor::Visit(E->IgnoreImplicit());
+ }
+
+ // Any sort of decl reference, we just use the unqualified name.
+ std::string VisitMemberExpr(const MemberExpr *E) {
+ return getSimpleName(*E->getMemberDecl()).str();
+ }
+ std::string VisitDeclRefExpr(const DeclRefExpr *E) {
+ return getSimpleName(*E->getFoundDecl()).str();
+ }
+ std::string VisitCallExpr(const CallExpr *E) {
+ return Visit(E->getCallee());
+ }
+ std::string
+ VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+ return getSimpleName(E->getMember()).str();
+ }
+ std::string
+ VisitDependentScopeMemberExpr(const DependentScopeDeclRefExpr *E) {
+ return getSimpleName(E->getDeclName()).str();
+ }
+ std::string VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *E) {
+ return getSimpleName(E->getType()).str();
+ }
+ std::string VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E) {
+ return getSimpleName(E->getType()).str();
+ }
+
+ // Step through implicit nodes that clang doesn't classify as such.
+ std::string VisitCXXMemberCallExpr(const CXXMemberCallExpr *E) {
+ // Call to operator bool() inside if (X): dispatch to X.
+ if (E->getNumArgs() == 0 &&
+ E->getMethodDecl()->getDeclName().getNameKind() ==
+ DeclarationName::CXXConversionFunctionName &&
+ E->getSourceRange() ==
+ E->getImplicitObjectArgument()->getSourceRange())
+ return Visit(E->getImplicitObjectArgument());
+ return ConstStmtVisitor::VisitCXXMemberCallExpr(E);
+ }
+ std::string VisitCXXConstructExpr(const CXXConstructExpr *E) {
+ if (E->getNumArgs() == 1)
+ return Visit(E->getArg(0));
+ return "";
+ }
+
+ // Literals are just printed
+ std::string VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) {
+ return E->getValue() ? "true" : "false";
+ }
+ std::string VisitIntegerLiteral(const IntegerLiteral *E) {
+ return llvm::to_string(E->getValue());
+ }
+ std::string VisitFloatingLiteral(const FloatingLiteral *E) {
+ std::string Result;
+ llvm::raw_string_ostream OS(Result);
+ E->getValue().print(OS);
+ // Printer adds newlines?!
+ Result.resize(llvm::StringRef(Result).rtrim().size());
+ return Result;
+ }
+ std::string VisitStringLiteral(const StringLiteral *E) {
+ std::string Result = "\"";
+ if (E->containsNonAscii()) {
+ Result += "...";
+ } else if (E->getLength() > 10) {
+ Result += E->getString().take_front(7);
+ Result += "...";
+ } else {
+ llvm::raw_string_ostream OS(Result);
+ llvm::printEscapedString(E->getString(), OS);
+ }
+ Result.push_back('"');
+ return Result;
+ }
+
+ // Simple operators. Motivating cases are `!x` and `I < Length`.
+ std::string printUnary(llvm::StringRef Spelling, const Expr *Operand,
+ bool Prefix) {
+ std::string Sub = Visit(Operand);
+ if (Sub.empty())
+ return "";
+ if (Prefix)
+ return (Spelling + Sub).str();
+ Sub += Spelling;
+ return Sub;
+ }
+ bool InsideBinary = false; // No recursing into binary expressions.
+ std::string printBinary(llvm::StringRef Spelling, const Expr *LHSOp,
+ const Expr *RHSOp) {
+ if (InsideBinary)
+ return "";
+ llvm::SaveAndRestore InBinary(InsideBinary, true);
+
+ std::string LHS = Visit(LHSOp);
+ std::string RHS = Visit(RHSOp);
+ if (LHS.empty() && RHS.empty())
+ return "";
+
+ if (LHS.empty())
+ LHS = "...";
+ LHS.push_back(' ');
+ LHS += Spelling;
+ LHS.push_back(' ');
+ if (RHS.empty())
+ LHS += "...";
+ else
+ LHS += RHS;
+ return LHS;
+ }
+ std::string VisitUnaryOperator(const UnaryOperator *E) {
+ return printUnary(E->getOpcodeStr(E->getOpcode()), E->getSubExpr(),
+ !E->isPostfix());
+ }
+ std::string VisitBinaryOperator(const BinaryOperator *E) {
+ return printBinary(E->getOpcodeStr(E->getOpcode()), E->getLHS(),
+ E->getRHS());
+ }
+ std::string VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *E) {
+ const char *Spelling = getOperatorSpelling(E->getOperator());
+ // Handle weird unary-that-look-like-binary postfix operators.
+ if ((E->getOperator() == OO_PlusPlus ||
+ E->getOperator() == OO_MinusMinus) &&
+ E->getNumArgs() == 2)
+ return printUnary(Spelling, E->getArg(0), false);
+ if (E->isInfixBinaryOp())
+ return printBinary(Spelling, E->getArg(0), E->getArg(1));
+ if (E->getNumArgs() == 1) {
+ switch (E->getOperator()) {
+ case OO_Plus:
+ case OO_Minus:
+ case OO_Star:
+ case OO_Amp:
+ case OO_Tilde:
+ case OO_Exclaim:
+ case OO_PlusPlus:
+ case OO_MinusMinus:
+ return printUnary(Spelling, E->getArg(0), true);
+ default:
+ break;
+ }
+ }
+ return "";
+ }
+ };
+ return Namer{}.Visit(E);
+}
+
// Determines if any intermediate type in desugaring QualType QT is of
// substituted template parameter type. Ignore pointer or reference wrappers.
bool isSugaredTemplateParameter(QualType QT) {
@@ -336,6 +545,66 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return true;
}
+ bool VisitForStmt(ForStmt *S) {
+ if (Cfg.InlayHints.BlockEnd) {
+ std::string Name;
+ // Common case: for (int I = 0; I < N; I++). Use "I" as the name.
+ if (auto *DS = llvm::dyn_cast_or_null<DeclStmt>(S->getInit());
+ DS && DS->isSingleDecl())
+ Name = getSimpleName(llvm::cast<NamedDecl>(*DS->getSingleDecl()));
+ else
+ Name = summarizeExpr(S->getCond());
+ markBlockEnd(S->getBody(), "for", Name);
+ }
+ return true;
+ }
+
+ bool VisitCXXForRangeStmt(CXXForRangeStmt *S) {
+ if (Cfg.InlayHints.BlockEnd)
+ markBlockEnd(S->getBody(), "for", getSimpleName(*S->getLoopVariable()));
+ return true;
+ }
+
+ bool VisitWhileStmt(WhileStmt *S) {
+ if (Cfg.InlayHints.BlockEnd)
+ markBlockEnd(S->getBody(), "while", summarizeExpr(S->getCond()));
+ return true;
+ }
+
+ bool VisitSwitchStmt(SwitchStmt *S) {
+ if (Cfg.InlayHints.BlockEnd)
+ markBlockEnd(S->getBody(), "switch", summarizeExpr(S->getCond()));
+ return true;
+ }
+
+ // If/else chains are tricky.
+ // if (cond1) {
+ // } else if (cond2) {
+ // } // mark as "cond1" or "cond2"?
+ // For now, the answer is neither, just mark as "if".
+ // The ElseIf is a
diff erent IfStmt that doesn't know about the outer one.
+ llvm::DenseSet<const IfStmt *> ElseIfs; // not eligible for names
+ bool VisitIfStmt(IfStmt *S) {
+ if (Cfg.InlayHints.BlockEnd) {
+ if (const auto *ElseIf = llvm::dyn_cast_or_null<IfStmt>(S->getElse()))
+ ElseIfs.insert(ElseIf);
+ // Don't use markBlockEnd: the relevant range is [then.begin, else.end].
+ if (const auto *EndCS = llvm::dyn_cast<CompoundStmt>(
+ S->getElse() ? S->getElse() : S->getThen())) {
+ addBlockEndHint(
+ {S->getThen()->getBeginLoc(), EndCS->getRBracLoc()}, "if",
+ ElseIfs.contains(S) ? "" : summarizeExpr(S->getCond()), "");
+ }
+ }
+ return true;
+ }
+
+ void markBlockEnd(const Stmt *Body, llvm::StringRef Label,
+ llvm::StringRef Name = "") {
+ if (const auto *CS = llvm::dyn_cast_or_null<CompoundStmt>(Body))
+ addBlockEndHint(CS->getSourceRange(), Label, Name, "");
+ }
+
bool VisitTagDecl(TagDecl *D) {
if (Cfg.InlayHints.BlockEnd && D->isThisDeclarationADefinition()) {
std::string DeclPrefix = D->getKindName().str();
@@ -683,18 +952,6 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return nullptr;
}
- static void stripLeadingUnderscores(StringRef &Name) {
- Name = Name.ltrim('_');
- }
-
- static StringRef getSimpleName(const NamedDecl &D) {
- if (IdentifierInfo *Ident = D.getDeclName().getAsIdentifierInfo()) {
- return Ident->getName();
- }
-
- return StringRef();
- }
-
// We pass HintSide rather than SourceLocation because we want to ensure
// it is in the same file as the common file range.
void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 9eb652983d39b7..73aec4cdf2ff61 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -17,6 +17,8 @@
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -1757,6 +1759,221 @@ TEST(BlockEndHints, Types) {
ExpectedHint{" // enum class E2", "E2"});
}
+TEST(BlockEndHints, If) {
+ assertBlockEndHints(
+ R"cpp(
+ void foo(bool cond) {
+ if (cond)
+ ;
+
+ if (cond) {
+ $simple[[}]]
+
+ if (cond) {
+ } else {
+ $ifelse[[}]]
+
+ if (cond) {
+ } else if (!cond) {
+ $elseif[[}]]
+
+ if (cond) {
+ } else {
+ if (!cond) {
+ $inner[[}]]
+ $outer[[}]]
+
+ if (auto X = cond) {
+ $init[[}]]
+
+ if (int i = 0; i > 10) {
+ $init_cond[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // if cond", "simple"},
+ ExpectedHint{" // if cond", "ifelse"}, ExpectedHint{" // if", "elseif"},
+ ExpectedHint{" // if !cond", "inner"},
+ ExpectedHint{" // if cond", "outer"}, ExpectedHint{" // if X", "init"},
+ ExpectedHint{" // if i > 10", "init_cond"});
+}
+
+TEST(BlockEndHints, Loops) {
+ assertBlockEndHints(
+ R"cpp(
+ void foo() {
+ while (true)
+ ;
+
+ while (true) {
+ $while[[}]]
+
+ do {
+ } while (true);
+
+ for (;true;) {
+ $forcond[[}]]
+
+ for (int I = 0; I < 10; ++I) {
+ $forvar[[}]]
+
+ int Vs[] = {1,2,3};
+ for (auto V : Vs) {
+ $foreach[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // while true", "while"},
+ ExpectedHint{" // for true", "forcond"},
+ ExpectedHint{" // for I", "forvar"},
+ ExpectedHint{" // for V", "foreach"});
+}
+
+TEST(BlockEndHints, Switch) {
+ assertBlockEndHints(
+ R"cpp(
+ void foo(int I) {
+ switch (I) {
+ case 0: break;
+ $switch[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // switch I", "switch"});
+}
+
+TEST(BlockEndHints, PrintLiterals) {
+ assertBlockEndHints(
+ R"cpp(
+ void foo() {
+ while ("foo") {
+ $string[[}]]
+
+ while ("foo but this time it is very long") {
+ $string_long[[}]]
+
+ while (true) {
+ $boolean[[}]]
+
+ while (1) {
+ $integer[[}]]
+
+ while (1.5) {
+ $float[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // while \"foo\"", "string"},
+ ExpectedHint{" // while \"foo but...\"", "string_long"},
+ ExpectedHint{" // while true", "boolean"},
+ ExpectedHint{" // while 1", "integer"},
+ ExpectedHint{" // while 1.5", "float"});
+}
+
+TEST(BlockEndHints, PrintRefs) {
+ assertBlockEndHints(
+ R"cpp(
+ namespace ns {
+ int Var;
+ int func();
+ struct S {
+ int Field;
+ int method() const;
+ }; // suppress
+ } // suppress
+ void foo() {
+ while (ns::Var) {
+ $var[[}]]
+
+ while (ns::func()) {
+ $func[[}]]
+
+ while (ns::S{}.Field) {
+ $field[[}]]
+
+ while (ns::S{}.method()) {
+ $method[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // while Var", "var"},
+ ExpectedHint{" // while func", "func"},
+ ExpectedHint{" // while Field", "field"},
+ ExpectedHint{" // while method", "method"});
+}
+
+TEST(BlockEndHints, PrintConversions) {
+ assertBlockEndHints(
+ R"cpp(
+ struct S {
+ S(int);
+ S(int, int);
+ explicit operator bool();
+ }; // suppress
+ void foo(int I) {
+ while (float(I)) {
+ $convert_primitive[[}]]
+
+ while (S(I)) {
+ $convert_class[[}]]
+
+ while (S(I, I)) {
+ $construct_class[[}]]
+ } // suppress
+ )cpp",
+ ExpectedHint{" // while float", "convert_primitive"},
+ ExpectedHint{" // while S", "convert_class"},
+ ExpectedHint{" // while S", "construct_class"});
+}
+
+TEST(BlockEndHints, PrintOperators) {
+ std::string AnnotatedCode = R"cpp(
+ void foo(Integer I) {
+ while(++I){
+ $preinc[[}]]
+
+ while(I++){
+ $postinc[[}]]
+
+ while(+(I + I)){
+ $unary_complex[[}]]
+
+ while(I < 0){
+ $compare[[}]]
+
+ while((I + I) < I){
+ $lhs_complex[[}]]
+
+ while(I < (I + I)){
+ $rhs_complex[[}]]
+
+ while((I + I) < (I + I)){
+ $binary_complex[[}]]
+ } // suppress
+ )cpp";
+
+ // We can't store shared expectations in a vector, assertHints uses varargs.
+ auto AssertExpectedHints = [&](llvm::StringRef Code) {
+ assertBlockEndHints(Code, ExpectedHint{" // while ++I", "preinc"},
+ ExpectedHint{" // while I++", "postinc"},
+ ExpectedHint{" // while", "unary_complex"},
+ ExpectedHint{" // while I < 0", "compare"},
+ ExpectedHint{" // while ... < I", "lhs_complex"},
+ ExpectedHint{" // while I < ...", "rhs_complex"},
+ ExpectedHint{" // while", "binary_complex"});
+ };
+
+ // First with built-in operators.
+ AssertExpectedHints("using Integer = int;" + AnnotatedCode);
+ // And now with overloading!
+ AssertExpectedHints(R"cpp(
+ struct Integer {
+ explicit operator bool();
+ Integer operator++();
+ Integer operator++(int);
+ Integer operator+(Integer);
+ Integer operator+();
+ bool operator<(Integer);
+ bool operator<(int);
+ }; // suppress
+ )cpp" + AnnotatedCode);
+}
+
TEST(BlockEndHints, TrailingSemicolon) {
assertBlockEndHints(R"cpp(
// The hint is placed after the trailing ';'
More information about the cfe-commits
mailing list