[clang-tools-extra] fe7afcf - [clangd] Remove inline Specifier for DefineOutline Tweak
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Fri May 26 01:49:17 PDT 2023
Author: Brian Gluzman
Date: 2023-05-26T10:43:08+02:00
New Revision: fe7afcf70c93223a16ec7a2a5e07c4ace16c9a04
URL: https://github.com/llvm/llvm-project/commit/fe7afcf70c93223a16ec7a2a5e07c4ace16c9a04
DIFF: https://github.com/llvm/llvm-project/commit/fe7afcf70c93223a16ec7a2a5e07c4ace16c9a04.diff
LOG: [clangd] Remove inline Specifier for DefineOutline Tweak
`inline` specifiers should be removed from from the function declaration and
the newly-created implementation.
For example, take the following (working) code:
```cpp
// foo.hpp
struct A {
inline void foo() { std::cout << "hello world\n" << std::flush; }
};
// foo.cpp
#include "foo.hpp"
// main.cpp
#include "foo.hpp"
int main() {
A a;
a.foo();
return 0;
}
// compile: clang++ -std=c++20 main.cpp foo.cpp -o main
```
After applying the tweak:
```
// foo.hpp
struct A {
inline void foo();
};
// foo.cpp
#include "foo.hpp"
inline void A::foo() { std::cout << "hello world\n" << std::flush; }
// main.cpp
#include "foo.hpp"
int main() {
A a;
a.foo();
return 0;
}
// compile: clang++ -std=c++20 main.cpp foo.cpp -o main
```
We get a link error, as expected:
```
/usr/bin/ld: /tmp/main-4c5d99.o: in function `main':
main.cpp:(.text+0x14): undefined reference to `A::foo()'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
This revision removes these specifiers from both the header and the source file. This was identified in Github issue llvm/llvm-project#61295.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D151294
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index f883397aaaf1e..b84ae04072f2c 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -131,6 +131,47 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
}
+// Returns replacements to delete tokens with kind `Kind` in the range
+// `FromRange`. Removes matching instances of given token preceeding the
+// function defition.
+llvm::Expected<tooling::Replacements>
+deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
+ SourceRange FromRange) {
+ tooling::Replacements DelKeywordCleanups;
+ llvm::Error Errors = llvm::Error::success();
+ bool FoundAny = false;
+ for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
+ if (Tok.kind() != Kind)
+ continue;
+ FoundAny = true;
+ auto Spelling = TokBuf.spelledForExpanded(llvm::ArrayRef(Tok));
+ if (!Spelling) {
+ Errors = llvm::joinErrors(
+ std::move(Errors),
+ error("define outline: couldn't remove `{0}` keyword.",
+ tok::getKeywordSpelling(Kind)));
+ break;
+ }
+ auto &SM = TokBuf.sourceManager();
+ CharSourceRange DelRange =
+ syntax::Token::range(SM, Spelling->front(), Spelling->back())
+ .toCharRange(SM);
+ if (auto Err =
+ DelKeywordCleanups.add(tooling::Replacement(SM, DelRange, "")))
+ Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+ }
+ if (!FoundAny) {
+ Errors = llvm::joinErrors(
+ std::move(Errors),
+ error("define outline: couldn't find `{0}` keyword to remove.",
+ tok::getKeywordSpelling(Kind)));
+ }
+
+ if (Errors)
+ return std::move(Errors);
+ return DelKeywordCleanups;
+}
+
// Creates a modified version of function definition that can be inserted at a
//
diff erent location, qualifies return value and function name to achieve that.
// Contains function signature, except defaulted parameter arguments, body and
@@ -251,34 +292,16 @@ getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
DelAttr(FD->getAttr<FinalAttr>());
auto DelKeyword = [&](tok::TokenKind Kind, SourceRange FromRange) {
- bool FoundAny = false;
- for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
- if (Tok.kind() != Kind)
- continue;
- FoundAny = true;
- auto Spelling = TokBuf.spelledForExpanded(llvm::ArrayRef(Tok));
- if (!Spelling) {
- Errors = llvm::joinErrors(
- std::move(Errors),
- error("define outline: couldn't remove `{0}` keyword.",
- tok::getKeywordSpelling(Kind)));
- break;
- }
- CharSourceRange DelRange =
- syntax::Token::range(SM, Spelling->front(), Spelling->back())
- .toCharRange(SM);
- if (auto Err =
- DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
- Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
- }
- if (!FoundAny) {
- Errors = llvm::joinErrors(
- std::move(Errors),
- error("define outline: couldn't find `{0}` keyword to remove.",
- tok::getKeywordSpelling(Kind)));
+ auto DelKeywords = deleteTokensWithKind(TokBuf, Kind, FromRange);
+ if (!DelKeywords) {
+ Errors = llvm::joinErrors(std::move(Errors), DelKeywords.takeError());
+ return;
}
+ DeclarationCleanups = DeclarationCleanups.merge(*DelKeywords);
};
+ if (FD->isInlineSpecified())
+ DelKeyword(tok::kw_inline, {FD->getBeginLoc(), FD->getLocation()});
if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
if (MD->isVirtualAsWritten())
DelKeyword(tok::kw_virtual, {FD->getBeginLoc(), FD->getLocation()});
@@ -460,15 +483,23 @@ class DefineOutline : public Tweak {
if (!Effect)
return Effect.takeError();
- // FIXME: We should also get rid of inline qualifier.
- const tooling::Replacement DeleteFuncBody(
+ tooling::Replacements HeaderUpdates(tooling::Replacement(
Sel.AST->getSourceManager(),
CharSourceRange::getTokenRange(*toHalfOpenFileRange(
SM, Sel.AST->getLangOpts(),
getDeletionRange(Source, Sel.AST->getTokens()))),
- ";");
- auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(),
- tooling::Replacements(DeleteFuncBody));
+ ";"));
+
+ if (Source->isInlineSpecified()) {
+ auto DelInline =
+ deleteTokensWithKind(Sel.AST->getTokens(), tok::kw_inline,
+ {Source->getBeginLoc(), Source->getLocation()});
+ if (!DelInline)
+ return DelInline.takeError();
+ HeaderUpdates = HeaderUpdates.merge(*DelInline);
+ }
+
+ auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
if (!HeaderFE)
return HeaderFE.takeError();
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index fd6d4d72e63ec..576377a658678 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -136,6 +136,12 @@ TEST_F(DefineOutlineTest, ApplyTest) {
"void foo() ;",
"void foo() { return; }",
},
+ // Inline specifier.
+ {
+ "inline void fo^o() { return; }",
+ " void foo() ;",
+ " void foo() { return; }",
+ },
// Default args.
{
"void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
@@ -319,6 +325,17 @@ TEST_F(DefineOutlineTest, ApplyTest) {
};)cpp",
" Foo::Foo(int) {}\n",
},
+ {
+ R"cpp(
+ struct A {
+ inline void f^oo(int) {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ void foo(int) ;
+ };)cpp",
+ " void A::foo(int) {}\n",
+ },
// Destrctors
{
"class A { ~A^(){} };",
More information about the cfe-commits
mailing list