diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -17,7 +17,7 @@ DumpAST.cpp DefineInline.cpp DefineOutline.cpp - ExpandAutoType.cpp + ExpandDeducedType.cpp ExpandMacro.cpp ExtractFunction.cpp ExtractVariable.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp rename from clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp rename to clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp @@ -1,4 +1,4 @@ -//===--- ExpandAutoType.cpp --------------------------------------*- C++-*-===// +//===--- ExpandDeducedType.cpp -----------------------------------*- C++-*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -29,8 +29,14 @@ /// After: /// MyClass x = Something(); /// ^^^^^^^ -/// FIXME: Handle decltype as well -class ExpandAutoType : public Tweak { +/// Expand `decltype(expr)` to the deduced type +/// Before: +/// decltype(0) i; +/// ^^^^^^^^^^^ +/// After: +/// int i; +/// ^^^ +class ExpandDeducedType : public Tweak { public: const char *id() const final; llvm::StringLiteral kind() const override { @@ -41,12 +47,14 @@ std::string title() const override; private: - SourceRange AutoRange; + SourceRange Range; }; -REGISTER_TWEAK(ExpandAutoType) +REGISTER_TWEAK(ExpandDeducedType) -std::string ExpandAutoType::title() const { return "Expand auto type"; } +std::string ExpandDeducedType::title() const { + return "Replace with deduced type"; +} // Structured bindings must use auto, e.g. `const auto& [a,b,c] = ...;`. // Return whether N (an AutoTypeLoc) is such an auto that must not be expanded. @@ -58,6 +66,13 @@ return N && N->ASTNode.get(); } +bool isLambda(QualType QT) { + if (!QT.isNull()) + if (const auto *RD = QT->getAsRecordDecl()) + return RD->isLambda(); + return false; +} + // Returns true iff Node is a lambda, and thus should not be expanded. Loc is // the location of the auto type. bool isDeducedAsLambda(const SelectionTree::Node *Node, SourceLocation Loc) { @@ -68,10 +83,9 @@ for (const auto *It = Node; It; It = It->Parent) { if (const auto *DD = It->ASTNode.get()) { if (DD->getTypeSourceInfo() && - DD->getTypeSourceInfo()->getTypeLoc().getBeginLoc() == Loc) { - if (auto *RD = DD->getType()->getAsRecordDecl()) - return RD->isLambda(); - } + DD->getTypeSourceInfo()->getTypeLoc().getBeginLoc() == Loc && + isLambda(DD->getType())) + return true; } } return false; @@ -86,14 +100,14 @@ return false; } -bool ExpandAutoType::prepare(const Selection &Inputs) { +bool ExpandDeducedType::prepare(const Selection &Inputs) { if (auto *Node = Inputs.ASTSelection.commonAncestor()) { if (auto *TypeNode = Node->ASTNode.get()) { if (const AutoTypeLoc Result = TypeNode->getAs()) { if (!isStructuredBindingType(Node) && !isDeducedAsLambda(Node, Result.getBeginLoc()) && !isTemplateParam(Node)) - AutoRange = Result.getSourceRange(); + Range = Result.getSourceRange(); } if (auto TTPAuto = TypeNode->getAs()) { // We exclude concept constraints for now, as the SourceRange is wrong. @@ -102,41 +116,53 @@ // TTPAuto->getSourceRange only covers "auto", not "C auto". if (TTPAuto.getDecl()->isImplicit() && !TTPAuto.getDecl()->hasTypeConstraint()) - AutoRange = TTPAuto.getSourceRange(); + Range = TTPAuto.getSourceRange(); + } + + if (auto DTTL = TypeNode->getAs()) { + if (!isLambda(cast(DTTL.getType())->getUnderlyingType())) + Range = DTTL.getSourceRange(); } } } - return AutoRange.isValid(); + return Range.isValid(); } -Expected ExpandAutoType::apply(const Selection& Inputs) { +Expected ExpandDeducedType::apply(const Selection &Inputs) { auto &SrcMgr = Inputs.AST->getSourceManager(); std::optional DeducedType = - getDeducedType(Inputs.AST->getASTContext(), AutoRange.getBegin()); + getDeducedType(Inputs.AST->getASTContext(), Range.getBegin()); // if we can't resolve the type, return an error message if (DeducedType == std::nullopt || (*DeducedType)->isUndeducedAutoType()) return error("Could not deduce type for 'auto' type"); - // if it's a lambda expression, return an error message - if (isa(*DeducedType) && - cast(*DeducedType)->getDecl()->isLambda()) { - return error("Could not expand type of lambda expression"); - } - - // if it's a function expression, return an error message - // naively replacing 'auto' with the type will break declarations. - // FIXME: there are other types that have similar problems - if (DeducedType->getTypePtr()->isFunctionPointerType()) { - return error("Could not expand type of function pointer"); - } - - std::string PrettyTypeName = printType(*DeducedType, - Inputs.ASTSelection.commonAncestor()->getDeclContext()); - - tooling::Replacement Expansion(SrcMgr, CharSourceRange(AutoRange, true), + // we shouldn't replace a dependent type which is likely not to print + // usefully, e.g. + // template + // struct Foobar { + // decltype(T{}) foobar; + // ^^^^^^^^^^^^^ would turn out to be `` + // }; + if ((*DeducedType)->isDependentType()) + return error("Could not expand a dependent type"); + + // Some types aren't written as single chunks of text, e.g: + // auto fptr = &func; // auto is void(*)() + // ==> + // void (*fptr)() = &func; + // Replacing these requires examining the declarator, we don't support it yet. + std::string PrettyDeclarator = printType( + *DeducedType, Inputs.ASTSelection.commonAncestor()->getDeclContext(), + "DECLARATOR_ID"); + llvm::StringRef PrettyTypeName = PrettyDeclarator; + if (!PrettyTypeName.consume_back("DECLARATOR_ID")) + return error("Could not expand type that isn't a simple string"); + PrettyTypeName = PrettyTypeName.rtrim(); + + tooling::Replacement Expansion(SrcMgr, CharSourceRange(Range, true), PrettyTypeName); return Effect::mainFileEdit(SrcMgr, tooling::Replacements(Expansion)); diff --git a/clang-tools-extra/clangd/test/check-fail.test b/clang-tools-extra/clangd/test/check-fail.test --- a/clang-tools-extra/clangd/test/check-fail.test +++ b/clang-tools-extra/clangd/test/check-fail.test @@ -7,7 +7,7 @@ // CHECK: [pp_file_not_found] Line {{.*}}: 'missing.h' file not found // CHECK: Building AST... // CHECK: Testing features at each token -// CHECK: tweak: ExpandAutoType ==> FAIL +// CHECK: tweak: ExpandDeducedType ==> FAIL // CHECK: All checks completed, 2 errors #include "missing.h" diff --git a/clang-tools-extra/clangd/test/check-lines.test b/clang-tools-extra/clangd/test/check-lines.test --- a/clang-tools-extra/clangd/test/check-lines.test +++ b/clang-tools-extra/clangd/test/check-lines.test @@ -7,7 +7,7 @@ // CHECK: Building preamble... // CHECK: Building AST... // CHECK: Testing features at each token -// CHECK: tweak: ExpandAutoType ==> FAIL +// CHECK: tweak: ExpandDeducedType ==> FAIL // CHECK: All checks completed, 1 errors void fun(); diff --git a/clang-tools-extra/clangd/test/check.test b/clang-tools-extra/clangd/test/check.test --- a/clang-tools-extra/clangd/test/check.test +++ b/clang-tools-extra/clangd/test/check.test @@ -7,7 +7,7 @@ // CHECK: Built preamble // CHECK: Building AST... // CHECK: Testing features at each token -// CHECK-DAG: tweak: ExpandAuto +// CHECK-DAG: tweak: ExpandDeducedType // CHECK-DAG: hover: true // CHECK-DAG: tweak: AddUsing // CHECK: All checks completed, 0 errors diff --git a/clang-tools-extra/clangd/test/code-action-request.test b/clang-tools-extra/clangd/test/code-action-request.test --- a/clang-tools-extra/clangd/test/code-action-request.test +++ b/clang-tools-extra/clangd/test/code-action-request.test @@ -43,11 +43,11 @@ # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, -# CHECK-NEXT: "tweakID": "ExpandAutoType" +# CHECK-NEXT: "tweakID": "ExpandDeducedType" # CHECK-NEXT: } # CHECK-NEXT: ], # CHECK-NEXT: "command": "clangd.applyTweak", -# CHECK-NEXT: "title": "Expand auto type" +# CHECK-NEXT: "title": "Replace with deduced type" # CHECK-NEXT: } # CHECK-NEXT: ] --- @@ -92,7 +92,7 @@ # CHECK-NEXT: "result": [ # CHECK-NEXT: { --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "newText": "int", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/test/request-reply.test b/clang-tools-extra/clangd/test/request-reply.test --- a/clang-tools-extra/clangd/test/request-reply.test +++ b/clang-tools-extra/clangd/test/request-reply.test @@ -3,7 +3,7 @@ --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}} --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "id": 0, # CHECK: "method": "workspace/applyEdit", # CHECK: "newText": "int", @@ -25,7 +25,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "id": 4, --- -{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "id": 1, # CHECK: "method": "workspace/applyEdit", --- diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -119,7 +119,7 @@ tweaks/DumpASTTests.cpp tweaks/DumpRecordLayoutTests.cpp tweaks/DumpSymbolTests.cpp - tweaks/ExpandAutoTypeTests.cpp + tweaks/ExpandDeducedTypeTests.cpp tweaks/ExpandMacroTests.cpp tweaks/ExtractFunctionTests.cpp tweaks/ExtractVariableTests.cpp diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp rename from clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp rename to clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp @@ -1,4 +1,4 @@ -//===-- ExpandAutoTypeTests.cpp ---------------------------------*- C++ -*-===// +//===-- ExpandDeducedTypeTests.cpp ------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -16,9 +16,9 @@ namespace clangd { namespace { -TWEAK_TEST(ExpandAutoType); +TWEAK_TEST(ExpandDeducedType); -TEST_F(ExpandAutoTypeTest, Test) { +TEST_F(ExpandDeducedTypeTest, Test) { Header = R"cpp( namespace ns { struct Class { @@ -50,7 +50,10 @@ StartsWith("fail: Could not deduce type for 'auto' type")); // function pointers should not be replaced EXPECT_THAT(apply("au^to x = &ns::Func;"), - StartsWith("fail: Could not expand type of function pointer")); + StartsWith("fail: Could not expand type")); + // function references should not be replaced + EXPECT_THAT(apply("au^to &x = ns::Func;"), + StartsWith("fail: Could not expand type")); // lambda types are not replaced EXPECT_UNAVAILABLE("au^to x = []{};"); // inline namespaces @@ -59,9 +62,12 @@ // local class EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"), "namespace x { void y() { struct S{}; S z = S(); } }"); - // replace array types + // replace pointers EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"), R"cpp(const char * x = "test";)cpp"); + // pointers to an array are not replaced + EXPECT_THAT(apply(R"cpp(au^to s = &"foobar";)cpp"), + StartsWith("fail: Could not expand type")); EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"), "ns::Class * foo() { ns::Class * c = foo(); }"); @@ -71,6 +77,15 @@ EXPECT_EQ(apply("dec^ltype(auto) x = 10;"), "int x = 10;"); EXPECT_EQ(apply("decltype(au^to) x = 10;"), "int x = 10;"); + // references to array types are not replaced + EXPECT_THAT(apply(R"cpp(decl^type(auto) s = "foobar"; // error-ok)cpp"), + StartsWith("fail: Could not expand type")); + // array types are not replaced + EXPECT_THAT(apply("int arr[10]; decl^type(auto) foobar = arr; // error-ok"), + StartsWith("fail: Could not expand type")); + // pointers to an array are not replaced + EXPECT_THAT(apply(R"cpp(decl^type(auto) s = &"foobar";)cpp"), + StartsWith("fail: Could not expand type")); // expanding types in structured bindings is syntactically invalid. EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};"); @@ -78,6 +93,40 @@ EXPECT_THAT(apply("template void x() { ^auto y = T::z(); }"), StartsWith("fail: Could not deduce type for 'auto' type")); + // check primitive type + EXPECT_EQ(apply("decl^type(0) i;"), "int i;"); + // function should not be replaced + EXPECT_THAT(apply("void f(); decl^type(f) g;"), + StartsWith("fail: Could not expand type")); + // check return type in function proto + EXPECT_EQ(apply("decl^type(0) f();"), "int f();"); + // check trailing return type + EXPECT_EQ(apply("auto f() -> decl^type(0) { return 0; }"), + "auto f() -> int { return 0; }"); + // check function parameter type + EXPECT_EQ(apply("void f(decl^type(0));"), "void f(int);"); + // check template parameter type + EXPECT_EQ(apply("template struct Foobar {};"), + "template struct Foobar {};"); + // check default template argument + EXPECT_EQ(apply("template class Foo {};"), + "template class Foo {};"); + // check template argument + EXPECT_EQ(apply("template class Bar {}; Bar b;"), + "template class Bar {}; Bar b;"); + // dependent types are not replaced + EXPECT_THAT(apply("template struct Foobar { decl^type(T{}) t; };"), + StartsWith("fail: Could not expand a dependent type")); + // references to array types are not replaced + EXPECT_THAT(apply(R"cpp(decl^type("foobar") s; // error-ok)cpp"), + StartsWith("fail: Could not expand type")); + // array types are not replaced + EXPECT_THAT(apply("int arr[10]; decl^type(arr) foobar;"), + StartsWith("fail: Could not expand type")); + // pointers to an array are not replaced + EXPECT_THAT(apply(R"cpp(decl^type(&"foobar") s;)cpp"), + StartsWith("fail: Could not expand type")); + ExtraArgs.push_back("-std=c++20"); EXPECT_UNAVAILABLE("template class Y;"); @@ -90,6 +139,10 @@ // FIXME: should work on constrained auto params, once SourceRange is fixed. EXPECT_UNAVAILABLE("template concept C = true;" "auto X = [](C ^auto *){return 0;};"); + + // lambda should not be replaced + EXPECT_UNAVAILABLE("auto f = [](){}; decl^type(f) g;"); + EXPECT_UNAVAILABLE("decl^type([]{}) f;"); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h +++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h @@ -25,9 +25,9 @@ // Fixture base for testing tweaks. Intended to be subclassed for each tweak. // // Usage: -// TWEAK_TEST(ExpandAutoType); +// TWEAK_TEST(ExpandDeducedType); // -// TEST_F(ExpandAutoTypeTest, ShortensTypes) { +// TEST_F(ExpandDeducedTypeTest, ShortensTypes) { // Header = R"cpp( // namespace foo { template class X{}; } // using namespace foo; diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -1611,7 +1611,7 @@ clang-tools-extra/clangd/unittests/tweaks/DumpASTTests.cpp clang-tools-extra/clangd/unittests/tweaks/DumpRecordLayoutTests.cpp clang-tools-extra/clangd/unittests/tweaks/DumpSymbolTests.cpp -clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp +clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp clang-tools-extra/clangd/unittests/tweaks/ExpandMacroTests.cpp clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn --- a/llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn +++ b/llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn @@ -18,7 +18,7 @@ "DefineInline.cpp", "DefineOutline.cpp", "DumpAST.cpp", - "ExpandAutoType.cpp", + "ExpandDeducedType.cpp", "ExpandMacro.cpp", "ExtractFunction.cpp", "ExtractVariable.cpp", diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn --- a/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn +++ b/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn @@ -130,7 +130,7 @@ "tweaks/DumpASTTests.cpp", "tweaks/DumpRecordLayoutTests.cpp", "tweaks/DumpSymbolTests.cpp", - "tweaks/ExpandAutoTypeTests.cpp", + "tweaks/ExpandDeducedTypeTests.cpp", "tweaks/ExpandMacroTests.cpp", "tweaks/ExtractFunctionTests.cpp", "tweaks/ExtractVariableTests.cpp",