sammccall created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
sammccall planned changes to this revision.
sammccall added a comment.
This needs tests, but wanted to share the ideas.
The problem:
LSP specifies that Positions are between characters. Therefore when a position
(or an empty range) is used to target elements of the source code, there is an
ambiguity - should we look left or right of the cursor?
Until now, SelectionTree resolved this to the right except in trivial cases
(where there's whitespace, semicolon, or eof on the right).
This meant that it's unable to e.g. out-line `int foo^()` today.
Complicating this, LSP notwithstanding the cursor is *on* a character in many
editors (mostly terminal-based). In these cases there's no ambiguity - we must
"look right" - but there's also no way to tell in LSP.
(Several features currently resolve this by using getBeginningOfIdentifier,
which tries to rewind and supports end-of-identifier. But this relies on
raw lexing and is limited and buggy).
Precedent: well - most other languages aren't so full of densely packed symbols
that we might want to target. Bias-towards-identifier works well enough.
MS C++ for vscode seems to mostly use bias-toward-identifier too.
The problem with this solution is it doesn't provide any way to target some
things such as the constructor call in Foo^(bar());
Presented solution:
When an ambiguous selection is found, we generate *both* possible selection
trees. We try to run the feature on the rightward tree first, and then on the
leftward tree if it fails.
This is basically do-what-I-mean, the main downside is the need to do this on
a feature-by-feature basis (because each feature knows what "fail" means).
The most complicated instance of this is Tweaks, where the preferred selection
may vary tweak-by-tweak.
Wrinkles:
While production behavior is pretty consistent, this introduces some
inconsistency in testing, depending whether the interface we're testing is
inside or outside the "retry" wrapper.
In particular, for many features like Hover, the unit tests will show production
behavior, while for Tweaks the harness would have to run the loop itself if
we want this.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71345
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/Selection.h
clang-tools-extra/clangd/SemanticSelection.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang/include/clang/Tooling/Syntax/Tokens.h
clang/lib/Tooling/Syntax/Tokens.cpp
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -647,3 +647,18 @@
}
return OS.str();
}
+
+llvm::ArrayRef<syntax::Token>
+syntax::spelledTokensTouching(SourceLocation Loc,
+ const syntax::TokenBuffer &Tokens) {
+ assert(Loc.isFileID());
+ llvm::ArrayRef<syntax::Token> All =
+ Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+ // SourceLocation < SourceLocation is OK for one file ID.
+ auto *Right = llvm::partition_point(All, [&](const syntax::Token &Tok) {
+ return Tok.location() < Loc;
+ });
+ bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+ bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() == Loc;
+ return llvm::makeArrayRef(Right - int(AcceptLeft), Right + int(AcceptRight));
+}
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===================================================================
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -309,6 +309,11 @@
const SourceManager *SourceMgr;
};
+/// Return the spelled tokens that overlap or touch spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef<syntax::Token>
+spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
+
/// Lex the text buffer, corresponding to \p FID, in raw mode and record the
/// resulting spelled tokens. Does minimal post-processing on raw identifiers,
/// setting the appropriate token kind (instead of the raw_identifier reported
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -76,7 +76,10 @@
TU.ExtraArgs = ExtraArgs;
TU.AdditionalFiles = std::move(ExtraFiles);
ParsedAST AST = TU.build();
- Tweak::Selection S(Index, AST, Selection.first, Selection.second);
+ Tweak::Selection S(
+ Index, AST, Selection.first, Selection.second,
+ SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+ Selection.first, Selection.second));
auto PrepareResult = prepareTweak(TweakID, S);
if (PrepareResult)
return true;
@@ -99,7 +102,10 @@
TU.Code = Input.code();
TU.ExtraArgs = ExtraArgs;
ParsedAST AST = TU.build();
- Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
+ Tweak::Selection S(
+ Index.get(), AST, Selection.first, Selection.second,
+ SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+ Selection.first, Selection.second));
auto T = prepareTweak(TweakID, S);
if (!T) {
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -22,17 +22,20 @@
SelectionTree makeSelectionTree(const StringRef MarkedCode, ParsedAST &AST) {
Annotations Test(MarkedCode);
switch (Test.points().size()) {
- case 1: // Point selection.
- return SelectionTree(AST.getASTContext(), AST.getTokens(),
- cantFail(positionToOffset(Test.code(), Test.point())));
+ case 1: { // Point selection.
+ unsigned Offset = cantFail(positionToOffset(Test.code(), Test.point()));
+ return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+ Offset, Offset);
+ }
case 2: // Range selection.
- return SelectionTree(
+ return SelectionTree::createRight(
AST.getASTContext(), AST.getTokens(),
cantFail(positionToOffset(Test.code(), Test.points()[0])),
cantFail(positionToOffset(Test.code(), Test.points()[1])));
default:
ADD_FAILURE() << "Expected 1-2 points for selection.\n" << MarkedCode;
- return SelectionTree(AST.getASTContext(), AST.getTokens(), 0u, 0u);
+ return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0u,
+ 0u);
}
}
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -549,7 +549,7 @@
}
)cpp",
R"cpp(// Template auto parameter. Nothing (Not useful).
- template<^auto T>
+ template<a^uto T>
void func() {
}
void foo() {
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -77,8 +77,8 @@
auto AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
llvm::Annotations::Range R = A.range();
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,
- R.End);
+ auto Selection = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), R.Begin, R.End);
const SelectionTree::Node *N = Selection.commonAncestor();
if (!N) {
ADD_FAILURE() << "No node selected!\n" << Code;
Index: clang-tools-extra/clangd/refactor/Tweak.h
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -48,7 +48,7 @@
/// Input to prepare and apply tweaks.
struct Selection {
Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
- unsigned RangeEnd);
+ unsigned RangeEnd, SelectionTree ASTSelection);
/// The text of the active document.
llvm::StringRef Code;
/// The Index for handling codebase related queries.
Index: clang-tools-extra/clangd/refactor/Tweak.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.cpp
+++ clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -46,10 +46,10 @@
} // namespace
Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
- unsigned RangeBegin, unsigned RangeEnd)
+ unsigned RangeBegin, unsigned RangeEnd,
+ SelectionTree ASTSelection)
: Index(Index), AST(AST), SelectionBegin(RangeBegin),
- SelectionEnd(RangeEnd),
- ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) {
+ SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
auto &SM = AST.getSourceManager();
Code = SM.getBufferData(SM.getMainFileID());
Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -78,7 +78,8 @@
unsigned Offset =
AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+ SelectionTree Selection = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), Offset, Offset);
const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
if (!SelectedNode)
return {};
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -134,11 +134,16 @@
FileID FID;
unsigned Offset;
std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
std::vector<const Decl *> Result;
- if (const SelectionTree::Node *N = Selection.commonAncestor()) {
- auto Decls = targetDecl(N->ASTNode, Relations);
- Result.assign(Decls.begin(), Decls.end());
+ for (const SelectionTree &Selection : SelectionTree::create(
+ AST.getASTContext(), AST.getTokens(), Offset, Offset)) {
+ llvm::errs() << "1\n";
+ if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+ auto Decls = targetDecl(N->ASTNode, Relations);
+ Result.assign(Decls.begin(), Decls.end());
+ }
+ if (!Result.empty())
+ break;
}
return Result;
}
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -39,7 +39,8 @@
}
// Get node under the cursor.
- SelectionTree ST(AST.getASTContext(), AST.getTokens(), *Offset);
+ SelectionTree ST = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), *Offset, *Offset);
for (const auto *Node = ST.commonAncestor(); Node != nullptr;
Node = Node->Parent) {
if (const Decl *D = Node->ASTNode.get<Decl>()) {
Index: clang-tools-extra/clangd/Selection.h
===================================================================
--- clang-tools-extra/clangd/Selection.h
+++ clang-tools-extra/clangd/Selection.h
@@ -64,16 +64,32 @@
// point back into the AST it was constructed with.
class SelectionTree {
public:
- // Creates a selection tree at the given byte offset in the main file.
- // This is approximately equivalent to a range of one character.
- // (Usually, the character to the right of Offset, sometimes to the left).
- SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Offset);
- // Creates a selection tree for the given range in the main file.
- // The range includes bytes [Start, End).
- // If Start == End, uses the same heuristics as SelectionTree(AST, Start).
- SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Start, unsigned End);
+ // Create a selection tree for the given range.
+ //
+ // If the range is empty and borders two tokens, a tree for the right token
+ // and a tree for the left token will be returned.
+ // Callers should iterate over the result and use the first tree where the
+ // feature is available.
+ //
+ // If the range doesn't touch any tokens, returns one empty selection.
+ static llvm::SmallVector<SelectionTree, 2>
+ create(ASTContext &AST, const syntax::TokenBuffer &Tokens, unsigned Begin,
+ unsigned End);
+
+ // Create a selection tree for the given range.
+ //
+ // Where ambiguous (range is empty and borders two tokens), prefer the token
+ // on the right.
+ static SelectionTree createRight(ASTContext &AST,
+ const syntax::TokenBuffer &Tokens,
+ unsigned Begin, unsigned End);
+
+ // Copies are no good - contain pointers to other nodes.
+ SelectionTree(const SelectionTree &) = delete;
+ SelectionTree &operator=(const SelectionTree &) = delete;
+ // Moves are OK though - internal storage is pointer-stable when moved.
+ SelectionTree(SelectionTree &&) = default;
+ SelectionTree &operator=(SelectionTree &&) = default;
// Describes to what extent an AST node is covered by the selection.
enum Selection : unsigned char {
@@ -121,6 +137,11 @@
const Node &root() const { return *Root; }
private:
+ // Creates a selection tree for the given range in the main file.
+ // The range includes bytes [Start, End).
+ SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
+ unsigned Start, unsigned End);
+
std::deque<Node> Nodes; // Stable-pointer storage.
const Node *Root;
clang::PrintingPolicy PrintPolicy;
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -142,6 +142,11 @@
Result = SelectionTree::Partial;
}
+// As well as comments, don't count semicolons as real tokens.
+// They're not properly claimed as expr-statement is missing from the AST.
+bool shouldIgnore(const syntax::Token &Tok) {
+ return Tok.kind() == tok::comment || Tok.kind() == tok::semi;
+}
// SelectionTester can determine whether a range of tokens from the PP-expanded
// stream (corresponding to an AST node) is considered selected.
@@ -172,9 +177,7 @@
});
// Precompute selectedness and offset for selected spelled tokens.
for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) {
- // As well as comments, don't count semicolons as real tokens.
- // They're not properly claimed as expr-statement is missing from the AST.
- if (T->kind() == tok::comment || T->kind() == tok::semi)
+ if (shouldIgnore(*T))
continue;
SpelledTokens.emplace_back();
Tok &S = SpelledTokens.back();
@@ -650,24 +653,51 @@
return std::move(OS.str());
}
-// Decide which selection emulates a "point" query in between characters.
-static std::pair<unsigned, unsigned> pointBounds(unsigned Offset, FileID FID,
- ASTContext &AST) {
- StringRef Buf = AST.getSourceManager().getBufferData(FID);
- // Edge-cases where the choice is forced.
- if (Buf.size() == 0)
- return {0, 0};
- if (Offset == 0)
- return {0, 1};
- if (Offset == Buf.size())
- return {Offset - 1, Offset};
- // We could choose either this byte or the previous. Usually we prefer the
- // character on the right of the cursor (or under a block cursor).
- // But if that's whitespace/semicolon, we likely want the token on the left.
- auto IsIgnoredChar = [](char C) { return isWhitespace(C) || C == ';'; };
- if (IsIgnoredChar(Buf[Offset]) && !IsIgnoredChar(Buf[Offset - 1]))
- return {Offset - 1, Offset};
- return {Offset, Offset + 1};
+// Decide which selections emulate a "point" query in between characters.
+// If it's ambiguous (the neighboring characters are selectable tokens), returns
+// both possibilities in preference order.
+// If the point doesn't touch any tokens, returns an empty array.
+static llvm::SmallVector<std::pair<unsigned, unsigned>, 2>
+pointBounds(SourceLocation Loc, const syntax::TokenBuffer &Tokens) {
+ llvm::SmallVector<std::pair<unsigned, unsigned>, 2> Result;
+ // Prefer right token over left.
+ for (const syntax::Token &Tok :
+ llvm::reverse(spelledTokensTouching(Loc, Tokens))) {
+ if (shouldIgnore(Tok))
+ continue;
+ unsigned Offset = Tokens.sourceManager().getFileOffset(Tok.location());
+ Result.emplace_back(Offset, Offset + Tok.length());
+ }
+ return Result;
+}
+
+llvm::SmallVector<SelectionTree, 2>
+SelectionTree::create(ASTContext &AST, const syntax::TokenBuffer &Tokens,
+ unsigned int Begin, unsigned int End) {
+ llvm::SmallVector<SelectionTree, 2> Result;
+ if (Begin == End) {
+ const auto &SM = AST.getSourceManager();
+ for (std::pair<unsigned, unsigned> Bounds :
+ pointBounds(SM.getComposedLoc(SM.getMainFileID(), Begin), Tokens))
+ Result.push_back(SelectionTree(AST, Tokens, Bounds.first, Bounds.second));
+ }
+ if (Result.empty()) // nontrivial selection, or empty and uninteresting
+ Result.push_back(SelectionTree(AST, Tokens, Begin, End));
+ return Result;
+}
+
+SelectionTree SelectionTree::createRight(ASTContext &AST,
+ const syntax::TokenBuffer &Tokens,
+ unsigned int Begin, unsigned int End) {
+ const auto &SM = AST.getSourceManager();
+ if (Begin == End) {
+ auto Bounds = pointBounds(
+ SM.getComposedLoc(SM.getMainFileID(), Begin), Tokens);
+ if (!Bounds.empty())
+ std::tie(Begin, End) = Bounds.front();
+ // else continue and create an empty selection.
+ }
+ return SelectionTree(AST, Tokens, Begin, End);
}
SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
@@ -677,8 +707,6 @@
// but that's all clangd has needed so far.
const SourceManager &SM = AST.getSourceManager();
FileID FID = SM.getMainFileID();
- if (Begin == End)
- std::tie(Begin, End) = pointBounds(Begin, FID, AST);
PrintPolicy.TerseOutput = true;
PrintPolicy.IncludeNewlines = false;
@@ -690,10 +718,6 @@
dlog("Built selection tree\n{0}", *this);
}
-SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Offset)
- : SelectionTree(AST, Tokens, Offset, Offset) {}
-
const Node *SelectionTree::commonAncestor() const {
const Node *Ancestor = Root;
while (Ancestor->Children.size() == 1 && !Ancestor->Selected)
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -408,21 +408,25 @@
llvm::consumeError(Offset.takeError());
return llvm::None;
}
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
- std::vector<const Decl *> Result;
- if (const SelectionTree::Node *N = Selection.commonAncestor()) {
- DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
- auto Decls = targetDecl(N->ASTNode, Rel);
- if (!Decls.empty()) {
- HI = getHoverContents(Decls.front(), Index);
- // Look for a close enclosing expression to show the value of.
- if (!HI->Value)
- HI->Value = printExprValue(N, AST.getASTContext());
+ for (const SelectionTree &Selection : SelectionTree::create(
+ AST.getASTContext(), AST.getTokens(), *Offset, *Offset)) {
+ std::vector<const Decl *> Result;
+ if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+ DeclRelationSet Rel =
+ DeclRelation::TemplatePattern | DeclRelation::Alias;
+ auto Decls = targetDecl(N->ASTNode, Rel);
+ if (!Decls.empty()) {
+ HI = getHoverContents(Decls.front(), Index);
+ // Look for a close enclosing expression to show the value of.
+ if (!HI->Value)
+ HI->Value = printExprValue(N, AST.getASTContext());
+ break;
+ }
+ // FIXME: support hovers for other nodes?
+ // - certain expressions (sizeof etc)
+ // - built-in types
+ // - literals (esp user-defined)
}
- // FIXME: support hovers for other nodes?
- // - certain expressions (sizeof etc)
- // - built-in types
- // - literals (esp user-defined)
}
}
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -374,7 +374,8 @@
WorkScheduler.runWithAST("Rename", File, std::move(Action));
}
-static llvm::Expected<Tweak::Selection>
+// May generate several candidate selections, due to SelectionTree ambiguity.
+static llvm::Expected<std::vector<Tweak::Selection>>
tweakSelection(const Range &Sel, const InputsAndAST &AST) {
auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
if (!Begin)
@@ -382,7 +383,12 @@
auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
if (!End)
return End.takeError();
- return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
+ std::vector<Tweak::Selection> Result;
+ for (auto &ST : SelectionTree::create(AST.AST.getASTContext(),
+ AST.AST.getTokens(), *Begin, *End))
+ Result.emplace_back(AST.Inputs.Index, AST.AST, *Begin, *End, std::move(ST));
+ assert(!Result.empty() && "Expected at least one SelectionTree");
+ return Result;
}
void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
@@ -391,12 +397,21 @@
this](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selection = tweakSelection(Sel, *InpAST);
- if (!Selection)
- return CB(Selection.takeError());
+ auto Selections = tweakSelection(Sel, *InpAST);
+ if (!Selections)
+ return CB(Selections.takeError());
std::vector<TweakRef> Res;
- for (auto &T : prepareTweaks(*Selection, TweakFilter))
- Res.push_back({T->id(), T->title(), T->intent()});
+ // Don't allow a tweak to fire more than once across ambiguous selections.
+ llvm::DenseSet<llvm::StringRef> PreparedTweaks;
+ auto Filter = [&](const Tweak &T) {
+ return TweakFilter(T) && !PreparedTweaks.count(T.id());
+ };
+ for (const auto &Sel : *Selections) {
+ for (auto &T : prepareTweaks(Sel, Filter)) {
+ Res.push_back({T->id(), T->title(), T->intent()});
+ PreparedTweaks.insert(T->id());
+ }
+ }
CB(std::move(Res));
};
@@ -411,21 +426,31 @@
FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selection = tweakSelection(Sel, *InpAST);
- if (!Selection)
- return CB(Selection.takeError());
- auto A = prepareTweak(TweakID, *Selection);
- if (!A)
- return CB(A.takeError());
- auto Effect = (*A)->apply(*Selection);
- if (!Effect)
- return CB(Effect.takeError());
- for (auto &It : Effect->ApplyEdits) {
- Edit &E = It.second;
- format::FormatStyle Style =
- getFormatStyleForFile(File, E.InitialCode, FS.get());
- if (llvm::Error Err = reformatEdit(E, Style))
- elog("Failed to format {0}: {1}", It.first(), std::move(Err));
+ auto Selections = tweakSelection(Sel, *InpAST);
+ if (!Selections)
+ return CB(Selections.takeError());
+ llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;
+ // Try each selection, take the first one that prepare()s.
+ // If they all fail, Effect will hold get the last error.
+ for (const auto &Selection : *Selections) {
+ auto T = prepareTweak(TweakID, Selection);
+ if (T) {
+ Effect = (*T)->apply(Selection);
+ break;
+ } else {
+ Effect = T.takeError();
+ }
+ }
+ assert(Effect.hasValue() && "Expected at least one selection");
+ if (*Effect) {
+ // Tweaks don't apply clang-format, do that centrally here.
+ for (auto &It : (*Effect)->ApplyEdits) {
+ Edit &E = It.second;
+ format::FormatStyle Style =
+ getFormatStyleForFile(File, E.InitialCode, FS.get());
+ if (llvm::Error Err = reformatEdit(E, Style))
+ elog("Failed to format {0}: {1}", It.first(), std::move(Err));
+ }
}
return CB(std::move(*Effect));
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
