[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL357823: [Lexer] NFC: Fix an off-by-one bug in 
getAsCharRange(). (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59977?vs=192764&id=193974#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59977/new/

https://reviews.llvm.org/D59977

Files:
  cfe/trunk/include/clang/Basic/PlistSupport.h
  cfe/trunk/include/clang/Lex/Lexer.h
  cfe/trunk/unittests/Lex/LexerTest.cpp


Index: cfe/trunk/unittests/Lex/LexerTest.cpp
===================================================================
--- cfe/trunk/unittests/Lex/LexerTest.cpp
+++ cfe/trunk/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: cfe/trunk/include/clang/Lex/Lexer.h
===================================================================
--- cfe/trunk/include/clang/Lex/Lexer.h
+++ cfe/trunk/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: cfe/trunk/include/clang/Basic/PlistSupport.h
===================================================================
--- cfe/trunk/include/clang/Basic/PlistSupport.h
+++ cfe/trunk/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: cfe/trunk/unittests/Lex/LexerTest.cpp
===================================================================
--- cfe/trunk/unittests/Lex/LexerTest.cpp
+++ cfe/trunk/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: cfe/trunk/include/clang/Lex/Lexer.h
===================================================================
--- cfe/trunk/include/clang/Lex/Lexer.h
+++ cfe/trunk/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: cfe/trunk/include/clang/Basic/PlistSupport.h
===================================================================
--- cfe/trunk/include/clang/Basic/PlistSupport.h
+++ cfe/trunk/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";
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits