[PATCH] D105365: [Lexer] Fix bug in `makeFileCharRange` called on split tokens.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG93dc73b1e0f3: [Lexer] Fix bug in `makeFileCharRange` called 
on split tokens. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105365

Files:
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/LexerTest.cpp

Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -25,6 +25,7 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 
 namespace {
@@ -65,7 +66,7 @@
 
   std::vector Lex(StringRef Source) {
 TrivialModuleLoader ModLoader;
-auto PP = CreatePP(Source, ModLoader);
+PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
 while (1) {
@@ -109,6 +110,7 @@
   LangOptions LangOpts;
   std::shared_ptr TargetOpts;
   IntrusiveRefCntPtr Target;
+  std::unique_ptr PP;
 };
 
 TEST_F(LexerTest, GetSourceTextExpandsToMaximumInMacroArgument) {
@@ -264,12 +266,14 @@
 
 TEST_F(LexerTest, LexAPI) {
   std::vector ExpectedTokens;
+  // Line 1 (after the #defines)
   ExpectedTokens.push_back(tok::l_square);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::r_square);
   ExpectedTokens.push_back(tok::l_square);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::r_square);
+  // Line 2
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::identifier);
@@ -357,6 +361,65 @@
   EXPECT_EQ("N", Lexer::getImmediateMacroName(idLoc4, SourceMgr, LangOpts));
 }
 
+TEST_F(LexerTest, HandlesSplitTokens) {
+  std::vector ExpectedTokens;
+  // Line 1 (after the #defines)
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::greatergreater);
+  // Line 2
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::greatergreater);
+
+  std::vector toks = CheckLex("#define TY ty\n"
+ "#define RANGLE ty>\n"
+ "TY>\n"
+ "RANGLE",
+ ExpectedTokens);
+
+  SourceLocation outerTyLoc = toks[0].getLocation();
+  SourceLocation innerTyLoc = toks[2].getLocation();
+  SourceLocation gtgtLoc = toks[4].getLocation();
+  // Split the token to simulate the action of the parser and force creation of
+  // an `ExpansionTokenRange`.
+  SourceLocation rangleLoc = PP->SplitToken(gtgtLoc, 1);
+
+  // Verify that it only captures the first greater-then and not the second one.
+  CharSourceRange range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(innerTyLoc, rangleLoc), SourceMgr,
+  LangOpts);
+  EXPECT_TRUE(range.isCharRange());
+  EXPECT_EQ(range.getAsRange(),
+SourceRange(innerTyLoc, gtgtLoc.getLocWithOffset(1)));
+
+  // Verify case where range begins in a macro expansion.
+  range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(outerTyLoc, rangleLoc), SourceMgr,
+  LangOpts);
+  EXPECT_TRUE(range.isCharRange());
+  EXPECT_EQ(range.getAsRange(),
+SourceRange(SourceMgr.getExpansionLoc(outerTyLoc),
+gtgtLoc.getLocWithOffset(1)));
+
+  SourceLocation macroInnerTyLoc = toks[7].getLocation();
+  SourceLocation macroGtgtLoc = toks[9].getLocation();
+  // Split the token to simulate the action of the parser and force creation of
+  // an `ExpansionTokenRange`.
+  SourceLocation macroRAngleLoc = PP->SplitToken(macroGtgtLoc, 1);
+
+  // Verify that it fails (because it only captures the first greater-then and
+  // not the second one, so it doesn't span the entire macro expansion).
+  range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(macroInnerTyLoc, macroRAngleLoc),
+  SourceMgr, LangOpts);
+  EXPECT_TRUE(range.isInvalid());
+}
+
 TEST_F(LexerTest, DontMergeMacroArgsFromDifferentMacroFiles) {
   std::vector toks =
   Lex("#define helper1 0\n"
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -877,6 +877,14 @@
   return CharSourceRange::getCharRange(Begin, End);
 }
 
+// Assumes that `Loc` is in an expansion.
+static bool isInExpansionTokenRange(const SourceLocation Loc,
+const SourceManager ) {
+  return SM.getSLocEntry(SM.getFileID(Loc))
+  .getExpansion()
+  .isExpansionTokenRange();
+}
+
 CharSourceRange 

[PATCH] D105365: [Lexer] Fix bug in `makeFileCharRange` called on split tokens.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Sorry about this. I stared at this a few times and it *seems* right to me, and 
fixes the bug...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105365

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

[PATCH] D105365: [Lexer] Fix bug in `makeFileCharRange` called on split tokens.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Gentle ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105365

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

[PATCH] D105365: [Lexer] Fix bug in `makeFileCharRange` called on split tokens.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: sammccall.
ymandel requested review of this revision.
Herald added a project: clang.

When the end loc of the specified range is a split token, `makeFileCharRange`
does not process it correctly.  This patch adds proper support for split tokens.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105365

Files:
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/LexerTest.cpp

Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -25,6 +25,7 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 
 namespace {
@@ -65,7 +66,7 @@
 
   std::vector Lex(StringRef Source) {
 TrivialModuleLoader ModLoader;
-auto PP = CreatePP(Source, ModLoader);
+PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
 while (1) {
@@ -109,6 +110,7 @@
   LangOptions LangOpts;
   std::shared_ptr TargetOpts;
   IntrusiveRefCntPtr Target;
+  std::unique_ptr PP;
 };
 
 TEST_F(LexerTest, GetSourceTextExpandsToMaximumInMacroArgument) {
@@ -264,12 +266,14 @@
 
 TEST_F(LexerTest, LexAPI) {
   std::vector ExpectedTokens;
+  // Line 1 (after the #defines)
   ExpectedTokens.push_back(tok::l_square);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::r_square);
   ExpectedTokens.push_back(tok::l_square);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::r_square);
+  // Line 2
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::identifier);
   ExpectedTokens.push_back(tok::identifier);
@@ -357,6 +361,65 @@
   EXPECT_EQ("N", Lexer::getImmediateMacroName(idLoc4, SourceMgr, LangOpts));
 }
 
+TEST_F(LexerTest, HandlesSplitTokens) {
+  std::vector ExpectedTokens;
+  // Line 1 (after the #defines)
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::greatergreater);
+  // Line 2
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::identifier);
+  ExpectedTokens.push_back(tok::less);
+  ExpectedTokens.push_back(tok::greatergreater);
+
+  std::vector toks = CheckLex("#define TY ty\n"
+ "#define RANGLE ty>\n"
+ "TY>\n"
+ "RANGLE",
+ ExpectedTokens);
+
+  SourceLocation outerTyLoc = toks[0].getLocation();
+  SourceLocation innerTyLoc = toks[2].getLocation();
+  SourceLocation gtgtLoc = toks[4].getLocation();
+  // Split the token to simulate the action of the parser and force creation of
+  // an `ExpansionTokenRange`.
+  SourceLocation rangleLoc = PP->SplitToken(gtgtLoc, 1);
+
+  // Verify that it only captures the first greater-then and not the second one.
+  CharSourceRange range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(innerTyLoc, rangleLoc), SourceMgr,
+  LangOpts);
+  EXPECT_TRUE(range.isCharRange());
+  EXPECT_EQ(range.getAsRange(),
+SourceRange(innerTyLoc, gtgtLoc.getLocWithOffset(1)));
+
+  // Verify case where range begins in a macro expansion.
+  range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(outerTyLoc, rangleLoc), SourceMgr,
+  LangOpts);
+  EXPECT_TRUE(range.isCharRange());
+  EXPECT_EQ(range.getAsRange(),
+SourceRange(SourceMgr.getExpansionLoc(outerTyLoc),
+gtgtLoc.getLocWithOffset(1)));
+
+  SourceLocation macroInnerTyLoc = toks[7].getLocation();
+  SourceLocation macroGtgtLoc = toks[9].getLocation();
+  // Split the token to simulate the action of the parser and force creation of
+  // an `ExpansionTokenRange`.
+  SourceLocation macroRAngleLoc = PP->SplitToken(macroGtgtLoc, 1);
+
+  // Verify that it fails (because it only captures the first greater-then and
+  // not the second one, so it doesn't span the entire macro expansion).
+  range = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(macroInnerTyLoc, macroRAngleLoc),
+  SourceMgr, LangOpts);
+  EXPECT_TRUE(range.isInvalid());
+}
+
 TEST_F(LexerTest, DontMergeMacroArgsFromDifferentMacroFiles) {
   std::vector toks =
   Lex("#define helper1 0\n"
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -877,6 +877,14 @@
   return CharSourceRange::getCharRange(Begin, End);
 }
 
+// Assumes that `Loc` is in an expansion.
+static bool isInExpansionTokenRange(const SourceLocation Loc,
+const SourceManager ) {
+  return SM.getSLocEntry(SM.getFileID(Loc))
+  .getExpansion()
+