Skip to content

Commit 345c482

Browse files
authored
[libTooling] Fix getFileRangeForEdit for inner nested template types (#87673)
When there is `>>` in source from the right brackets of a nested template, the end location of the inner template points into a scratch space with a single `>` token. This prevents the lexer from seeing the `>>` token in the original source. However, this causes the range to appear to be partially in a macro, and is problematic if we are trying to avoid ranges with any macro expansions. This change detects these split tokens in token ranges, converting it to the corresponding character range without the expansion.
1 parent f0724f0 commit 345c482

File tree

2 files changed

+114
-5
lines changed

2 files changed

+114
-5
lines changed

clang/lib/Tooling/Transformer/SourceCode.cpp

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,54 @@ static bool spelledInMacroDefinition(SourceLocation Loc,
101101
return false;
102102
}
103103

104+
// Returns the expansion char-range of `Loc` if `Loc` is a split token. For
105+
// example, `>>` in nested templates needs the first `>` to be split, otherwise
106+
// the `SourceLocation` of the token would lex as `>>` instead of `>`.
107+
static std::optional<CharSourceRange>
108+
getExpansionForSplitToken(SourceLocation Loc, const SourceManager &SM,
109+
const LangOptions &LangOpts) {
110+
if (Loc.isMacroID()) {
111+
bool Invalid = false;
112+
auto &SLoc = SM.getSLocEntry(SM.getFileID(Loc), &Invalid);
113+
if (Invalid)
114+
return std::nullopt;
115+
if (auto &Expansion = SLoc.getExpansion();
116+
!Expansion.isExpansionTokenRange()) {
117+
// A char-range expansion is only used where a token-range would be
118+
// incorrect, and so identifies this as a split token (and importantly,
119+
// not as a macro).
120+
return Expansion.getExpansionLocRange();
121+
}
122+
}
123+
return std::nullopt;
124+
}
125+
126+
// If `Range` covers a split token, returns the expansion range, otherwise
127+
// returns `Range`.
128+
static CharSourceRange getRangeForSplitTokens(CharSourceRange Range,
129+
const SourceManager &SM,
130+
const LangOptions &LangOpts) {
131+
if (Range.isTokenRange()) {
132+
auto BeginToken = getExpansionForSplitToken(Range.getBegin(), SM, LangOpts);
133+
auto EndToken = getExpansionForSplitToken(Range.getEnd(), SM, LangOpts);
134+
if (EndToken) {
135+
SourceLocation BeginLoc =
136+
BeginToken ? BeginToken->getBegin() : Range.getBegin();
137+
// We can't use the expansion location with a token-range, because that
138+
// will incorrectly lex the end token, so use a char-range that ends at
139+
// the split.
140+
return CharSourceRange::getCharRange(BeginLoc, EndToken->getEnd());
141+
} else if (BeginToken) {
142+
// Since the end token is not split, the whole range covers the split, so
143+
// the only adjustment we make is to use the expansion location of the
144+
// begin token.
145+
return CharSourceRange::getTokenRange(BeginToken->getBegin(),
146+
Range.getEnd());
147+
}
148+
}
149+
return Range;
150+
}
151+
104152
static CharSourceRange getRange(const CharSourceRange &EditRange,
105153
const SourceManager &SM,
106154
const LangOptions &LangOpts,
@@ -109,13 +157,14 @@ static CharSourceRange getRange(const CharSourceRange &EditRange,
109157
if (IncludeMacroExpansion) {
110158
Range = Lexer::makeFileCharRange(EditRange, SM, LangOpts);
111159
} else {
112-
if (spelledInMacroDefinition(EditRange.getBegin(), SM) ||
113-
spelledInMacroDefinition(EditRange.getEnd(), SM))
160+
auto AdjustedRange = getRangeForSplitTokens(EditRange, SM, LangOpts);
161+
if (spelledInMacroDefinition(AdjustedRange.getBegin(), SM) ||
162+
spelledInMacroDefinition(AdjustedRange.getEnd(), SM))
114163
return {};
115164

116-
auto B = SM.getSpellingLoc(EditRange.getBegin());
117-
auto E = SM.getSpellingLoc(EditRange.getEnd());
118-
if (EditRange.isTokenRange())
165+
auto B = SM.getSpellingLoc(AdjustedRange.getBegin());
166+
auto E = SM.getSpellingLoc(AdjustedRange.getEnd());
167+
if (AdjustedRange.isTokenRange())
119168
E = Lexer::getLocForEndOfToken(E, 0, SM, LangOpts);
120169
Range = CharSourceRange::getCharRange(B, E);
121170
}

clang/unittests/Tooling/SourceCodeTest.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "clang/Tooling/Transformer/SourceCode.h"
1010
#include "TestVisitor.h"
11+
#include "clang/ASTMatchers/ASTMatchFinder.h"
1112
#include "clang/Basic/Diagnostic.h"
1213
#include "clang/Basic/SourceLocation.h"
1314
#include "clang/Lex/Lexer.h"
@@ -18,10 +19,12 @@
1819
#include <gtest/gtest.h>
1920

2021
using namespace clang;
22+
using namespace clang::ast_matchers;
2123

2224
using llvm::Failed;
2325
using llvm::Succeeded;
2426
using llvm::ValueIs;
27+
using testing::Optional;
2528
using tooling::getAssociatedRange;
2629
using tooling::getExtendedRange;
2730
using tooling::getExtendedText;
@@ -50,6 +53,15 @@ struct CallsVisitor : TestVisitor<CallsVisitor> {
5053
std::function<void(CallExpr *, ASTContext *Context)> OnCall;
5154
};
5255

56+
struct TypeLocVisitor : TestVisitor<TypeLocVisitor> {
57+
bool VisitTypeLoc(TypeLoc TL) {
58+
OnTypeLoc(TL, Context);
59+
return true;
60+
}
61+
62+
std::function<void(TypeLoc, ASTContext *Context)> OnTypeLoc;
63+
};
64+
5365
// Equality matcher for `clang::CharSourceRange`, which lacks `operator==`.
5466
MATCHER_P(EqualsRange, R, "") {
5567
return arg.isTokenRange() == R.isTokenRange() &&
@@ -510,6 +522,54 @@ int c = M3(3);
510522
Visitor.runOver(Code.code());
511523
}
512524

525+
TEST(SourceCodeTest, InnerNestedTemplate) {
526+
llvm::Annotations Code(R"cpp(
527+
template <typename T>
528+
struct A {};
529+
template <typename T>
530+
struct B {};
531+
template <typename T>
532+
struct C {};
533+
534+
void f(A<B<C<int>$r[[>>]]);
535+
)cpp");
536+
537+
TypeLocVisitor Visitor;
538+
Visitor.OnTypeLoc = [&](TypeLoc TL, ASTContext *Context) {
539+
if (TL.getSourceRange().isInvalid())
540+
return;
541+
542+
// There are no macros, so every TypeLoc's range should be valid.
543+
auto Range = CharSourceRange::getTokenRange(TL.getSourceRange());
544+
auto LastTokenRange = CharSourceRange::getTokenRange(TL.getEndLoc());
545+
EXPECT_TRUE(getFileRangeForEdit(Range, *Context,
546+
/*IncludeMacroExpansion=*/false))
547+
<< TL.getSourceRange().printToString(Context->getSourceManager());
548+
EXPECT_TRUE(getFileRangeForEdit(LastTokenRange, *Context,
549+
/*IncludeMacroExpansion=*/false))
550+
<< TL.getEndLoc().printToString(Context->getSourceManager());
551+
552+
if (auto matches = match(
553+
templateSpecializationTypeLoc(
554+
loc(templateSpecializationType(
555+
hasDeclaration(cxxRecordDecl(hasName("A"))))),
556+
hasTemplateArgumentLoc(
557+
0, templateArgumentLoc(hasTypeLoc(typeLoc().bind("b"))))),
558+
TL, *Context);
559+
!matches.empty()) {
560+
// A range where the start token is split, but the end token is not.
561+
auto OuterTL = TL;
562+
auto MiddleTL = *matches[0].getNodeAs<TypeLoc>("b");
563+
EXPECT_THAT(
564+
getFileRangeForEdit(CharSourceRange::getTokenRange(
565+
MiddleTL.getEndLoc(), OuterTL.getEndLoc()),
566+
*Context, /*IncludeMacroExpansion=*/false),
567+
Optional(EqualsAnnotatedRange(Context, Code.range("r"))));
568+
}
569+
};
570+
Visitor.runOver(Code.code(), TypeLocVisitor::Lang_CXX11);
571+
}
572+
513573
TEST_P(GetFileRangeForEditTest, EditPartialMacroExpansionShouldFail) {
514574
std::string Code = R"cpp(
515575
#define BAR 10+

0 commit comments

Comments
 (0)