v1nh1shungry updated this revision to Diff 487688. v1nh1shungry added a comment.
- avoid expanding dependent types - avoid expanding reference to a function - avoid expanding array type and reference to an array Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141226/new/ https://reviews.llvm.org/D141226 Files: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp clang-tools-extra/clangd/test/check-fail.test clang-tools-extra/clangd/test/check-lines.test clang-tools-extra/clangd/test/check.test clang-tools-extra/clangd/test/code-action-request.test clang-tools-extra/clangd/test/request-reply.test clang-tools-extra/clangd/unittests/CMakeLists.txt clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h clang/docs/tools/clang-formatted-files.txt llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn =================================================================== --- llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn +++ llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn @@ -129,7 +129,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", Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn =================================================================== --- llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn +++ 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", Index: clang/docs/tools/clang-formatted-files.txt =================================================================== --- clang/docs/tools/clang-formatted-files.txt +++ 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 Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h =================================================================== --- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h +++ 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<typename> class X{}; } // using namespace foo; Index: clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/tweaks/ExpandDeducedTypeTests.cpp +++ 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 of function")); + // function references should not be replaced + EXPECT_THAT(apply("au^to &x = ns::Func;"), + StartsWith("fail: Could not expand type of function")); // lambda types are not replaced EXPECT_UNAVAILABLE("au^to x = []{};"); // inline namespaces @@ -59,7 +62,7 @@ // 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 pointer to array EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"), R"cpp(const char * x = "test";)cpp"); @@ -71,6 +74,12 @@ 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 reference to an array")); + // array types are not replaced + EXPECT_THAT(apply("int arr[10]; decl^type(auto) foobar = arr; // error-ok"), + StartsWith("fail: Could not expand type of array")); // expanding types in structured bindings is syntactically invalid. EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};"); @@ -78,6 +87,34 @@ EXPECT_THAT(apply("template <typename T> 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 of function")); + // 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 default template argument + EXPECT_EQ(apply("template <class = decl^type(0)> class Foo {};"), + "template <class = int> class Foo {};"); + // check template argument + EXPECT_EQ(apply("template <class> class Bar {}; Bar<decl^type(0)> b;"), + "template <class> class Bar {}; Bar<int> b;"); + // dependent types are not replaced + EXPECT_THAT(apply("template <class T> 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 reference to an array")); + // array types are not replaced + EXPECT_THAT(apply("int arr[10]; decl^type(arr) foobar;"), + StartsWith("fail: Could not expand type of array")); + ExtraArgs.push_back("-std=c++20"); EXPECT_UNAVAILABLE("template <au^to X> class Y;"); @@ -90,6 +127,10 @@ // FIXME: should work on constrained auto params, once SourceRange is fixed. EXPECT_UNAVAILABLE("template<class> 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 Index: clang-tools-extra/clangd/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/unittests/CMakeLists.txt +++ clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -114,7 +114,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 Index: clang-tools-extra/clangd/test/request-reply.test =================================================================== --- clang-tools-extra/clangd/test/request-reply.test +++ 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", --- Index: clang-tools-extra/clangd/test/code-action-request.test =================================================================== --- clang-tools-extra/clangd/test/code-action-request.test +++ clang-tools-extra/clangd/test/code-action-request.test @@ -43,7 +43,7 @@ # 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", @@ -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": { Index: clang-tools-extra/clangd/test/check.test =================================================================== --- clang-tools-extra/clangd/test/check.test +++ 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 Index: clang-tools-extra/clangd/test/check-lines.test =================================================================== --- clang-tools-extra/clangd/test/check-lines.test +++ 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(); Index: clang-tools-extra/clangd/test/check-fail.test =================================================================== --- clang-tools-extra/clangd/test/check-fail.test +++ 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" Index: clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp =================================================================== --- clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp +++ 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,15 @@ std::string title() const override; private: - SourceRange AutoRange; + SourceRange Range; + bool IsAutoType; }; -REGISTER_TWEAK(ExpandAutoType) +REGISTER_TWEAK(ExpandDeducedType) -std::string ExpandAutoType::title() const { return "Expand auto type"; } +std::string ExpandDeducedType::title() const { + return IsAutoType ? "Expand auto type" : "Expand decltype"; +} // 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. @@ -86,14 +95,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<TypeLoc>()) { if (const AutoTypeLoc Result = TypeNode->getAs<AutoTypeLoc>()) { if (!isStructuredBindingType(Node) && !isDeducedAsLambda(Node, Result.getBeginLoc()) && !isTemplateParam(Node)) - AutoRange = Result.getSourceRange(); + Range = Result.getSourceRange(); } if (auto TTPAuto = TypeNode->getAs<TemplateTypeParmTypeLoc>()) { // We exclude concept constraints for now, as the SourceRange is wrong. @@ -102,41 +111,69 @@ // TTPAuto->getSourceRange only covers "auto", not "C auto". if (TTPAuto.getDecl()->isImplicit() && !TTPAuto.getDecl()->hasTypeConstraint()) - AutoRange = TTPAuto.getSourceRange(); + Range = TTPAuto.getSourceRange(); + } + IsAutoType = true; + + if (auto DTTL = TypeNode->getAs<DecltypeTypeLoc>()) { + if (!isDeducedAsLambda(Node, DTTL.getBeginLoc())) + Range = DTTL.getSourceRange(); + IsAutoType = false; } } } - return AutoRange.isValid(); + return Range.isValid(); } -Expected<Tweak::Effect> ExpandAutoType::apply(const Selection& Inputs) { +Expected<Tweak::Effect> ExpandDeducedType::apply(const Selection &Inputs) { auto &SrcMgr = Inputs.AST->getSourceManager(); std::optional<clang::QualType> 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<RecordType>(*DeducedType) && - cast<RecordType>(*DeducedType)->getDecl()->isLambda()) { - return error("Could not expand type of lambda expression"); + // we shouldn't replace a dependent type, e.g. + // template <class T> + // struct Foobar { + // decltype(T{}) foobar; + // }; + if ((*DeducedType)->isDependentType()) + return error("Could not expand a dependent type"); + + // we shouldn't replace an array type, e.g. + // int arr[10]; + // decltype(auto) foobar = arr; + // ^^^^^^^^^^^^^^ is `int[10]` + if ((*DeducedType)->isArrayType()) + return error("Could not expand type of array"); + + // we also shouldn't replace the reference to an array, e.g. + // decltype("foobar") s; + // ^^^^^^^^^^^^^^^^^ is `const char(&)[6]` + // and + // decltype(auto) s = "foobar"; + // ^^^^^^^^^^^^^^ is `const char(&)[6]` + if (const auto *RT = dyn_cast<ReferenceType>(*DeducedType)) { + if (RT->getPointeeType()->isArrayType()) + return error("Could not expand reference to an array"); } // 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"); + if ((*DeducedType)->isFunctionType() || + (*DeducedType)->isFunctionPointerType()) { + return error("Could not expand type of function"); } - std::string PrettyTypeName = printType(*DeducedType, - Inputs.ASTSelection.commonAncestor()->getDeclContext()); + std::string PrettyTypeName = printType( + *DeducedType, Inputs.ASTSelection.commonAncestor()->getDeclContext()); - tooling::Replacement Expansion(SrcMgr, CharSourceRange(AutoRange, true), + tooling::Replacement Expansion(SrcMgr, CharSourceRange(Range, true), PrettyTypeName); return Effect::mainFileEdit(SrcMgr, tooling::Replacements(Expansion)); Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits