llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd Author: Christian Kandeler (ckandeler) <details> <summary>Changes</summary> `decltype(x)` is rarely what a reader wants to see in a display context: a code completion offering `set_x(decltype(x) val)` conveys much less than `set_x(int val)`. clangd already worked around this for hover, by stripping decltypes off the type before printing it, with a FIXME noting that this belongs in a printing policy and that it does not handle composite types. `HoverTests.cpp` carries a matching FIXME on one of the cases it misses. Add such a policy flag, honour it in `TypePrinter`, and set it both for code completion and for the types clangd displays on hover. Applying it in the printer rather than at the call site also covers the cases the workaround could not reach: ```c++ const decltype(a) b; // was `int`, now `const int` void f(decltype(lamb) &bar); // was `decltype(lamb) &`, now `(lambda) &` auto f(decltype(a)) -> decltype(a); // now `auto (int) -> int` ``` The first of those was a bug: `castAs<DecltypeType>()` looks through qualifiers, so the workaround silently dropped the `const`. Since the printed type may now already be desugared, an "aka" suffix that would repeat it verbatim is suppressed. A dependent `decltype` has no resolved type to offer and is left as written. One consequence worth calling out for review: the declaration clangd shows next to the type is printed with a different policy and still reflects the source spelling, so the `decltype` remains visible on the hover card. Code completion and signature help have no such fallback, so there the resolved type replaces the spelling outright. That is the intent of the change, but it is the part most open to discussion, and it is separable from the hover cleanup if reviewers would prefer to take only the latter. Assisted-by: Claude Opus 5 --- Full diff: https://github.com/llvm/llvm-project/pull/223950.diff 7 Files Affected: - (modified) clang-tools-extra/clangd/Hover.cpp (+11-6) - (modified) clang-tools-extra/clangd/unittests/HoverTests.cpp (+3-6) - (modified) clang-tools-extra/docs/ReleaseNotes.md (+12) - (modified) clang/docs/ReleaseNotes.md (+5) - (modified) clang/include/clang/AST/PrettyPrinter.h (+10-2) - (modified) clang/lib/AST/TypePrinter.cpp (+8-1) - (modified) clang/lib/Sema/SemaCodeComplete.cpp (+1) ``````````diff diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index a2f8b6418833d..b7e851e805e1f 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -166,11 +166,6 @@ const char *getMarkdownLanguage(const ASTContext &Ctx) { HoverInfo::PrintedType printType(QualType QT, ASTContext &ASTCtx, const PrintingPolicy &PP) { - // TypePrinter doesn't resolve decltypes, so resolve them here. - // FIXME: This doesn't handle composite types that contain a decltype in them. - // We should rather have a printing policy for that. - while (!QT.isNull() && QT->isDecltypeType()) - QT = QT->castAs<DecltypeType>()->getUnderlyingType(); HoverInfo::PrintedType Result; llvm::raw_string_ostream OS(Result.Type); // Special case: if the outer type is a canonical tag type, then include the @@ -178,6 +173,10 @@ HoverInfo::PrintedType printType(QualType QT, ASTContext &ASTCtx, // complex cases, including pointers/references, template specializations, // etc. PrintingPolicy Copy(PP); + // Show what a decltype resolves to; `int` is more useful than `decltype(x)`. + // Unlike the declaration printed as HI.Definition, this is not meant to + // reflect how the type was spelled. + Copy.ResolveDecltype = true; if (!QT.isNull() && !QT.hasQualifiers() && PP.SuppressTagKeyword) { if (auto *TT = llvm::dyn_cast<TagType>(QT.getTypePtr()); TT && TT->isCanonicalUnqualified()) { @@ -191,8 +190,14 @@ HoverInfo::PrintedType printType(QualType QT, ASTContext &ASTCtx, if (!QT.isNull() && Cfg.Hover.ShowAKA) { bool ShouldAKA = false; QualType DesugaredTy = clang::desugarForDiagnostic(ASTCtx, QT, ShouldAKA); - if (ShouldAKA) + if (ShouldAKA) { Result.AKA = DesugaredTy.getAsString(Copy); + // ShouldAKA reflects desugaring at the AST level, but the printing + // policy may already have resolved the difference away (e.g. for a + // decltype). Don't print "int (aka int)". + if (Result.AKA == Result.Type) + Result.AKA.reset(); + } } return Result; } diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index d355118f30761..15b03e6bb6ece 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -471,7 +471,7 @@ class Foo final {})cpp"; HI.Name = "bar"; HI.Kind = index::SymbolKind::Parameter; HI.Definition = "decltype(lamb) &bar"; - HI.Type = {"decltype(lamb) &", "(lambda) &"}; + HI.Type = "(lambda) &"; HI.ReturnType = "bool"; HI.Parameters = { {{"int"}, std::string("T"), std::nullopt}, @@ -3000,7 +3000,7 @@ TEST(Hover, All) { HI.Kind = index::SymbolKind::Variable; HI.NamespaceScope = ""; HI.Name = "b"; - HI.Type = "int"; + HI.Type = "const int"; }}, { R"cpp(// type with decltype @@ -3011,10 +3011,7 @@ TEST(Hover, All) { HI.Kind = index::SymbolKind::Function; HI.NamespaceScope = ""; HI.Name = "foo"; - // FIXME: Handle composite types with decltype with a printing - // policy. - HI.Type = {"auto (decltype(a)) -> decltype(a)", - "auto (int) -> int"}; + HI.Type = "auto (int) -> int"; HI.ReturnType = "int"; HI.Parameters = {{{"int"}, std::string("x"), std::nullopt}}; }}, diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 98105ac222df9..b2da4046592c0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -81,8 +81,17 @@ infrastructure are described first, followed by tool-specific sections. #### Hover +- The type a `decltype` resolves to is now also shown for composite types, + e.g. `decltype(x)&` is displayed as `int&`. Qualifiers applied to a + `decltype` are no longer dropped, so `const decltype(x)` is displayed as + `const int` rather than `int`. + #### Code completion +- Parameters declared with a `decltype` are now displayed as the type the + `decltype` resolves to, e.g. `set_x(int val)` rather than + `set_x(decltype(x) val)`. + #### Code actions - clangd now applies clang-tidy fix-it post-processing before exposing fixes. @@ -94,6 +103,9 @@ infrastructure are described first, followed by tool-specific sections. #### Signature help +- Parameters declared with a `decltype` are now displayed as the type the + `decltype` resolves to, as for code completion. + #### Cross-references #### Objective-C diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 043a0ddae2a6c..db571dedacb1f 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -842,6 +842,11 @@ features cannot lower the translation-unit ABI level; ### Code Completion +- Parameters declared with a `decltype` are now presented as the type the + `decltype` resolves to, e.g. `set_x(int val)` rather than + `set_x(decltype(x) val)`. This affects the completion strings produced by + libclang as well as those used by clangd. + ### Static Analyzer #### Crash and bug fixes diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h index c6555215ce20a..bb4ee585fd837 100644 --- a/clang/include/clang/AST/PrettyPrinter.h +++ b/clang/include/clang/AST/PrettyPrinter.h @@ -92,8 +92,9 @@ struct PrintingPolicy { SuppressImplicitBase(false), FullyQualifiedName(false), PrintAsCanonical(false), PrintInjectedClassNameWithArguments(true), UsePreferredNames(true), AlwaysIncludeTypeForTemplateArgument(false), - CleanUglifiedParameters(false), EntireContentsOfLargeArray(true), - PrettyEnums(true), UseEnumerators(true), UseHLSLTypes(LO.HLSL), + CleanUglifiedParameters(false), ResolveDecltype(false), + EntireContentsOfLargeArray(true), PrettyEnums(true), + UseEnumerators(true), UseHLSLTypes(LO.HLSL), SuppressDeclAttributes(false), SuppressLambdaBody(false) {} /// Adjust this printing policy for cases where it's known that we're @@ -352,6 +353,13 @@ struct PrintingPolicy { LLVM_PREFERRED_TYPE(bool) unsigned CleanUglifiedParameters : 1; + /// Whether to print the type a non-dependent `decltype(expr)` resolves to, + /// rather than the `decltype` specifier itself. Intended for display + /// contexts such as code completion, where `int` is more informative than + /// `decltype(x)`; it does not describe how the type was spelled. + LLVM_PREFERRED_TYPE(bool) + unsigned ResolveDecltype : 1; + /// Whether to print the entire array initializers, especially on non-type /// template parameters, no matter how many elements there are. LLVM_PREFERRED_TYPE(bool) diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp index 80d63434c28b1..37c2b8ddf1da0 100644 --- a/clang/lib/AST/TypePrinter.cpp +++ b/clang/lib/AST/TypePrinter.cpp @@ -1359,6 +1359,10 @@ void TypePrinter::printTypeOfBefore(const TypeOfType *T, raw_ostream &OS) { void TypePrinter::printTypeOfAfter(const TypeOfType *T, raw_ostream &OS) {} void TypePrinter::printDecltypeBefore(const DecltypeType *T, raw_ostream &OS) { + if (Policy.ResolveDecltype && T->isSugared()) { + printBefore(T->desugar(), OS); + return; + } OS << "decltype("; if (const Expr *E = T->getUnderlyingExpr()) { PrintingPolicy ExprPolicy = Policy; @@ -1384,7 +1388,10 @@ void TypePrinter::printPackIndexingBefore(const PackIndexingType *T, void TypePrinter::printPackIndexingAfter(const PackIndexingType *T, raw_ostream &OS) {} -void TypePrinter::printDecltypeAfter(const DecltypeType *T, raw_ostream &OS) {} +void TypePrinter::printDecltypeAfter(const DecltypeType *T, raw_ostream &OS) { + if (Policy.ResolveDecltype && T->isSugared()) + printAfter(T->desugar(), OS); +} void TypePrinter::printUnaryTransformBefore(const UnaryTransformType *T, raw_ostream &OS) { diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 2bebbe6c79293..2e85228139528 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -2111,6 +2111,7 @@ static PrintingPolicy getCompletionPrintingPolicy(const ASTContext &Context, Policy.SuppressStrongLifetime = true; Policy.SuppressUnwrittenScope = true; Policy.CleanUglifiedParameters = true; + Policy.ResolveDecltype = true; return Policy; } `````````` </details> https://github.com/llvm/llvm-project/pull/223950 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
