Skip to content

Commit

Permalink
[clangd] support expanding decltype(expr)
Browse files Browse the repository at this point in the history
Enable the existing tweak `ExpandAutoType` to expand
`decltype(expr)`, e.g.

```
decltype(0) i;
```

will expand to

```
int i;
```

Therefore, rename the tweak `ExpandAutoType` to `ExpandDeducedType`.

This patch also fixes some nits,

* avoid replacing reference to a function
* avoid replacing array types and reference to an array

Fixes clangd/clangd#1456

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D141226
  • Loading branch information
v1nh1shungry authored and sam-mccall committed Jan 13, 2023
1 parent 1eaadae commit 1feb7af
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 53 deletions.
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ add_clang_library(clangDaemonTweaks OBJECT
DumpAST.cpp
DefineInline.cpp
DefineOutline.cpp
ExpandAutoType.cpp
ExpandDeducedType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
ExtractVariable.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -29,8 +29,14 @@ namespace {
/// 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 {
Expand All @@ -41,12 +47,14 @@ class ExpandAutoType : public Tweak {
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.
Expand All @@ -58,6 +66,13 @@ bool isStructuredBindingType(const SelectionTree::Node *N) {
return N && N->ASTNode.get<DecompositionDecl>();
}

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) {
Expand All @@ -68,10 +83,9 @@ bool isDeducedAsLambda(const SelectionTree::Node *Node, SourceLocation Loc) {
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;
Expand All @@ -86,14 +100,14 @@ bool isTemplateParam(const SelectionTree::Node *Node) {
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.
Expand All @@ -102,41 +116,53 @@ bool ExpandAutoType::prepare(const Selection &Inputs) {
// 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));
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/test/check-fail.test
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/test/check-lines.test
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/test/check.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions clang-tools-extra/clangd/test/code-action-request.test
Original file line number Diff line number Diff line change
Expand Up @@ -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: ]
---
Expand Down Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/test/request-reply.test
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
---
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/unittests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ add_unittest(ClangdUnitTests ClangdTests
tweaks/DumpASTTests.cpp
tweaks/DumpRecordLayoutTests.cpp
tweaks/DumpSymbolTests.cpp
tweaks/ExpandAutoTypeTests.cpp
tweaks/ExpandDeducedTypeTests.cpp
tweaks/ExpandMacroTests.cpp
tweaks/ExtractFunctionTests.cpp
tweaks/ExtractVariableTests.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -16,9 +16,9 @@ namespace clang {
namespace clangd {
namespace {

TWEAK_TEST(ExpandAutoType);
TWEAK_TEST(ExpandDeducedType);

TEST_F(ExpandAutoTypeTest, Test) {
TEST_F(ExpandDeducedTypeTest, Test) {
Header = R"cpp(
namespace ns {
struct Class {
Expand Down Expand Up @@ -50,7 +50,10 @@ TEST_F(ExpandAutoTypeTest, Test) {
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
Expand All @@ -59,9 +62,12 @@ TEST_F(ExpandAutoTypeTest, Test) {
// 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(); }");
Expand All @@ -71,13 +77,56 @@ TEST_F(ExpandAutoTypeTest, Test) {

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};");

// unknown types in a template should not be replaced
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;");

Expand All @@ -90,6 +139,10 @@ TEST_F(ExpandAutoTypeTest, Test) {
// 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
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ namespace clangd {
// 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;
Expand Down
2 changes: 1 addition & 1 deletion clang/docs/tools/clang-formatted-files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ source_set("tweaks") {
"DefineInline.cpp",
"DefineOutline.cpp",
"DumpAST.cpp",
"ExpandAutoType.cpp",
"ExpandDeducedType.cpp",
"ExpandMacro.cpp",
"ExtractFunction.cpp",
"ExtractVariable.cpp",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ unittest("ClangdTests") {
"tweaks/DumpASTTests.cpp",
"tweaks/DumpRecordLayoutTests.cpp",
"tweaks/DumpSymbolTests.cpp",
"tweaks/ExpandAutoTypeTests.cpp",
"tweaks/ExpandDeducedTypeTests.cpp",
"tweaks/ExpandMacroTests.cpp",
"tweaks/ExtractFunctionTests.cpp",
"tweaks/ExtractVariableTests.cpp",
Expand Down

0 comments on commit 1feb7af

Please sign in to comment.