NoQ updated this revision to Diff 196310.
NoQ marked an inline comment as done.
NoQ added a comment.
Add comments that `getAsCharRange()` is in fact used often misused.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59977/new/
https://reviews.llvm.org/D59977
Files:
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/unittests/clangd/SelectionTests.cpp
clang/include/clang/Basic/PlistSupport.h
clang/include/clang/Lex/Lexer.h
clang/unittests/Lex/LexerTest.cpp
Index: clang/unittests/Lex/LexerTest.cpp
===================================================================
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -513,4 +513,23 @@
EXPECT_EQ(String6, R"(a\\\n\n\n \\\\b)");
}
+TEST_F(LexerTest, CharRangeOffByOne) {
+ std::vector<Token> toks = Lex(R"(#define MOO 1
+ void foo() { MOO; })");
+ const Token &moo = toks[5];
+
+ EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+ SourceRange R{moo.getLocation(), moo.getLocation()};
+
+ EXPECT_TRUE(
+ Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+ EXPECT_TRUE(
+ Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+ CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+ EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
} // anonymous namespace
Index: clang/include/clang/Lex/Lexer.h
===================================================================
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -382,7 +382,7 @@
SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
}
static CharSourceRange getAsCharRange(CharSourceRange Range,
const SourceManager &SM,
Index: clang/include/clang/Basic/PlistSupport.h
===================================================================
--- clang/include/clang/Basic/PlistSupport.h
+++ clang/include/clang/Basic/PlistSupport.h
@@ -127,7 +127,11 @@
assert(R.isCharRange() && "cannot handle a token range");
Indent(o, indent) << "<array>\n";
EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
- EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+ // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
+ // in Lexer that is already fixed. It is here for backwards compatibility
+ // even though it is incorrect.
+ EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
Indent(o, indent) << "</array>\n";
}
Index: clang-tools-extra/unittests/clangd/SelectionTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/SelectionTests.cpp
+++ clang-tools-extra/unittests/clangd/SelectionTests.cpp
@@ -45,7 +45,7 @@
CharSourceRange R =
Lexer::getAsCharRange(SR, SM, AST.getASTContext().getLangOpts());
return Range{offsetToPosition(Buffer, SM.getFileOffset(R.getBegin())),
- offsetToPosition(Buffer, SM.getFileOffset(R.getEnd()) + 1)};
+ offsetToPosition(Buffer, SM.getFileOffset(R.getEnd()))};
}
std::string nodeKind(const SelectionTree::Node *N) {
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
@@ -102,11 +102,14 @@
}
}
+ // FIXME: This probably breaks on comments between the namespace and its '{'.
auto TextRange =
Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin, LBracketLocation),
Sources, getLangOpts());
StringRef NestedNamespaceName =
- Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
+ Lexer::getSourceText(TextRange, Sources, getLangOpts())
+ .rtrim('{') // Drop the { itself.
+ .rtrim(); // Drop any whitespace before it.
bool IsNested = NestedNamespaceName.contains(':');
if (IsNested)
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -39,6 +39,7 @@
const SourceRange &ReplacementRange,
const SourceManager &Sources,
const LangOptions &LangOpts) {
+ // FIXME: This logic breaks when there is a comment with ':'s in the middle.
CharSourceRange TextRange =
Lexer::getAsCharRange(ReplacementRange, Sources, LangOpts);
StringRef CurrentNamespacesText =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
