Skip to content

Commit 905b002

Browse files
committed
Revert "[clangd] Rethink how SelectionTree deals with macros and #includes."
This reverts commit 19daa21. It causes a bunch of failures on a bot that I've been unable to reproduce so far: http://45.33.8.238/mac/3308/step_7.txt
1 parent 26ab827 commit 905b002

File tree

7 files changed

+153
-391
lines changed

7 files changed

+153
-391
lines changed

clang-tools-extra/clangd/Selection.cpp

Lines changed: 135 additions & 279 deletions
Large diffs are not rendered by default.

clang-tools-extra/clangd/Selection.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class SelectionTree {
7676
unsigned Start, unsigned End);
7777

7878
// Describes to what extent an AST node is covered by the selection.
79-
enum Selection : unsigned char {
79+
enum Selection {
8080
// The AST node owns no characters covered by the selection.
8181
// Note that characters owned by children don't count:
8282
// if (x == 0) scream();

clang-tools-extra/clangd/unittests/SelectionTests.cpp

Lines changed: 3 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,6 @@ TEST(SelectionTest, CommonAncestor) {
134134
)cpp",
135135
"IfStmt",
136136
},
137-
{
138-
R"cpp(
139-
int x(int);
140-
#define M(foo) x(foo)
141-
int a = 42;
142-
int b = M([[^a]]);
143-
)cpp",
144-
"DeclRefExpr",
145-
},
146137
{
147138
R"cpp(
148139
void foo();
@@ -387,7 +378,6 @@ TEST(SelectionTest, Selected) {
387378
$C[[return]];
388379
}]] else [[{^
389380
}]]]]
390-
char z;
391381
}
392382
)cpp",
393383
R"cpp(
@@ -396,10 +386,10 @@ TEST(SelectionTest, Selected) {
396386
void foo(^$C[[unique_ptr<$C[[unique_ptr<$C[[int]]>]]>]]^ a) {}
397387
)cpp",
398388
R"cpp(int a = [[5 >^> 1]];)cpp",
399-
R"cpp(
389+
R"cpp([[
400390
#define ECHO(X) X
401-
ECHO(EC^HO($C[[int]]) EC^HO(a));
402-
)cpp",
391+
ECHO(EC^HO([[$C[[int]]) EC^HO(a]]));
392+
]])cpp",
403393
R"cpp( $C[[^$C[[int]] a^]]; )cpp",
404394
R"cpp( $C[[^$C[[int]] a = $C[[5]]^]]; )cpp",
405395
};
@@ -438,56 +428,6 @@ TEST(SelectionTest, PathologicalPreprocessor) {
438428
EXPECT_EQ("WhileStmt", T.commonAncestor()->Parent->kind());
439429
}
440430

441-
TEST(SelectionTest, IncludedFile) {
442-
const char *Case = R"cpp(
443-
void test() {
444-
#include "Exp^and.inc"
445-
break;
446-
}
447-
)cpp";
448-
Annotations Test(Case);
449-
auto TU = TestTU::withCode(Test.code());
450-
TU.AdditionalFiles["Expand.inc"] = "while(1)\n";
451-
auto AST = TU.build();
452-
auto T = makeSelectionTree(Case, AST);
453-
454-
EXPECT_EQ("WhileStmt", T.commonAncestor()->kind());
455-
}
456-
457-
TEST(SelectionTest, MacroArgExpansion) {
458-
// If a macro arg is expanded several times, we consider them all selected.
459-
const char *Case = R"cpp(
460-
int mul(int, int);
461-
#define SQUARE(X) mul(X, X);
462-
int nine = SQUARE(^3);
463-
)cpp";
464-
Annotations Test(Case);
465-
auto AST = TestTU::withCode(Test.code()).build();
466-
auto T = makeSelectionTree(Case, AST);
467-
// Unfortunately, this makes the common ancestor the CallExpr...
468-
// FIXME: hack around this by picking one?
469-
EXPECT_EQ("CallExpr", T.commonAncestor()->kind());
470-
EXPECT_FALSE(T.commonAncestor()->Selected);
471-
EXPECT_EQ(2u, T.commonAncestor()->Children.size());
472-
for (const auto* N : T.commonAncestor()->Children) {
473-
EXPECT_EQ("IntegerLiteral", N->kind());
474-
EXPECT_TRUE(N->Selected);
475-
}
476-
477-
// Verify that the common assert() macro doesn't suffer from this.
478-
// (This is because we don't associate the stringified token with the arg).
479-
Case = R"cpp(
480-
void die(const char*);
481-
#define assert(x) (x ? (void)0 : die(#x)
482-
void foo() { assert(^42); }
483-
)cpp";
484-
Test = Annotations(Case);
485-
AST = TestTU::withCode(Test.code()).build();
486-
T = makeSelectionTree(Case, AST);
487-
488-
EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
489-
}
490-
491431
TEST(SelectionTest, Implicit) {
492432
const char* Test = R"cpp(
493433
struct S { S(const char*); };

clang-tools-extra/clangd/unittests/TweakTests.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ TEST_F(ExtractVariableTest, Test) {
269269
EXPECT_UNAVAILABLE(UnavailableCases);
270270

271271
// vector of pairs of input and output strings
272-
const std::vector<std::pair<std::string, std::string>>
272+
const std::vector<std::pair<llvm::StringLiteral, llvm::StringLiteral>>
273273
InputOutputs = {
274274
// extraction from variable declaration/assignment
275275
{R"cpp(void varDecl() {
@@ -321,10 +321,17 @@ TEST_F(ExtractVariableTest, Test) {
321321
if(1)
322322
LOOP(5 + [[3]])
323323
})cpp",
324+
/*FIXME: It should be extracted like this. SelectionTree needs to be
325+
* fixed for macros.
324326
R"cpp(#define LOOP(x) while (1) {a = x;}
327+
void f(int a) {
328+
auto dummy = 3; if(1)
329+
LOOP(5 + dummy)
330+
})cpp"},*/
331+
R"cpp(#define LOOP(x) while (1) {a = x;}
325332
void f(int a) {
326-
auto dummy = 3; if(1)
327-
LOOP(5 + dummy)
333+
auto dummy = LOOP(5 + 3); if(1)
334+
dummy
328335
})cpp"},
329336
{R"cpp(#define LOOP(x) do {x;} while(1);
330337
void f(int a) {
@@ -637,18 +644,13 @@ void f(const int c) {
637644
)cpp";
638645
EXPECT_EQ(apply(TemplateFailInput), "unavailable");
639646

640-
std::string MacroInput = R"cpp(
647+
// FIXME: This should be extractable after selectionTree works correctly for
648+
// macros (currently it doesn't select anything for the following case)
649+
std::string MacroFailInput = R"cpp(
641650
#define F(BODY) void f() { BODY }
642651
F ([[int x = 0;]])
643652
)cpp";
644-
std::string MacroOutput = R"cpp(
645-
#define F(BODY) void f() { BODY }
646-
void extracted() {
647-
int x = 0;
648-
}
649-
F (extracted();)
650-
)cpp";
651-
EXPECT_EQ(apply(MacroInput), MacroOutput);
653+
EXPECT_EQ(apply(MacroFailInput), "unavailable");
652654

653655
// Shouldn't crash.
654656
EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");

clang/include/clang/Tooling/Syntax/Tokens.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,18 +175,13 @@ class TokenBuffer {
175175
/// All tokens produced by the preprocessor after all macro replacements,
176176
/// directives, etc. Source locations found in the clang AST will always
177177
/// point to one of these tokens.
178-
/// Tokens are in TU order (per SourceManager::isBeforeInTranslationUnit()).
179178
/// FIXME: figure out how to handle token splitting, e.g. '>>' can be split
180179
/// into two '>' tokens by the parser. However, TokenBuffer currently
181180
/// keeps it as a single '>>' token.
182181
llvm::ArrayRef<syntax::Token> expandedTokens() const {
183182
return ExpandedTokens;
184183
}
185184

186-
/// Returns the subrange of expandedTokens() corresponding to the closed
187-
/// token range R.
188-
llvm::ArrayRef<syntax::Token> expandedTokens(SourceRange R) const;
189-
190185
/// Find the subrange of spelled tokens that produced the corresponding \p
191186
/// Expanded tokens.
192187
///

clang/lib/Tooling/Syntax/Tokens.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -119,22 +119,6 @@ llvm::StringRef FileRange::text(const SourceManager &SM) const {
119119
return Text.substr(Begin, length());
120120
}
121121

122-
llvm::ArrayRef<syntax::Token> TokenBuffer::expandedTokens(SourceRange R) const {
123-
if (R.isInvalid())
124-
return {};
125-
const Token *Begin =
126-
llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {
127-
return SourceMgr->isBeforeInTranslationUnit(T.location(), R.getBegin());
128-
});
129-
const Token *End =
130-
llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {
131-
return !SourceMgr->isBeforeInTranslationUnit(R.getEnd(), T.location());
132-
});
133-
if (Begin > End)
134-
return {};
135-
return {Begin, End};
136-
}
137-
138122
std::pair<const syntax::Token *, const TokenBuffer::Mapping *>
139123
TokenBuffer::spelledForExpandedToken(const syntax::Token *Expanded) const {
140124
assert(Expanded);

clang/unittests/Tooling/Syntax/TokensTest.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
#include "llvm/Support/raw_ostream.h"
4141
#include "llvm/Testing/Support/Annotations.h"
4242
#include "llvm/Testing/Support/SupportHelpers.h"
43-
#include "gmock/gmock.h"
4443
#include <cassert>
4544
#include <cstdlib>
4645
#include <gmock/gmock.h>
@@ -664,20 +663,6 @@ TEST_F(TokenBufferTest, SpelledByExpanded) {
664663
ValueIs(SameRange(findSpelled("not_mapped"))));
665664
}
666665

667-
TEST_F(TokenBufferTest, ExpandedTokensForRange) {
668-
recordTokens(R"cpp(
669-
#define SIGN(X) X##_washere
670-
A SIGN(B) C SIGN(D) E SIGN(F) G
671-
)cpp");
672-
673-
SourceRange R(findExpanded("C").front().location(),
674-
findExpanded("F_washere").front().location());
675-
// Sanity check: expanded and spelled tokens are stored separately.
676-
EXPECT_THAT(Buffer.expandedTokens(R),
677-
SameRange(findExpanded("C D_washere E F_washere")));
678-
EXPECT_THAT(Buffer.expandedTokens(SourceRange()), testing::IsEmpty());
679-
}
680-
681666
TEST_F(TokenBufferTest, ExpansionStartingAt) {
682667
// Object-like macro expansions.
683668
recordTokens(R"cpp(

0 commit comments

Comments
 (0)