v1nh1shungry updated this revision to Diff 488297.
v1nh1shungry added a comment.
modify the comment
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"));
+ // 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 <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"));
+ // 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 <decl^type(0)> struct Foobar {};"),
+ "template <int> struct Foobar {};");
+ // 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 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 <au^to X> class Y;");
@@ -90,6 +139,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,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": {
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,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,12 @@
return N && N->ASTNode.get<DecompositionDecl>();
}
+bool isLambda(QualType QT) {
+ 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 +82,9 @@
for (const auto *It = Node; It; It = It->Parent) {
if (const auto *DD = It->ASTNode.get<DeclaratorDecl>()) {
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 +99,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 +115,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<DecltypeTypeLoc>()) {
+ if (!isLambda(cast<DecltypeType>(DTTL.getType())->getUnderlyingType()))
+ Range = DTTL.getSourceRange();
}
}
}
- 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");
- }
-
- // 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 <class T>
+ // struct Foobar {
+ // decltype(T{}) foobar;
+ // ^^^^^^^^^^^^^ would turn out to be `<dependent-type>`
+ // };
+ 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));
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
