[clang][lex] Expose findBeginningOfLine()

[KyleFromKitware]

Summary

And fix a few corner cases in which it returns the wrong answer.

[KyleFromKitware]

Fixed formatting.

[cor3ntin]

LGTM except the duplicated comment

clang/lib/Lex/Lexer.cpp

493–494

Should we remove the comment here?

[KyleFromKitware]

clang/lib/Lex/Lexer.cpp

493–494

Other Lexer methods follow a similar pattern of having doc comments both in the header and in the implementation. I think we can leave it.

[cor3ntin]

@KyleFromKitware oups, this fell under the cracks. I think I'm happy with is as-is, do you need help landing it?

[KyleFromKitware]

In D143099#4233623, @cor3ntin wrote:

@KyleFromKitware oups, this fell under the cracks. I think I'm happy with is as-is, do you need help landing it?

Yes, please.

[cor3ntin]

In D143099#4233633, @KyleFromKitware wrote:

In D143099#4233623, @cor3ntin wrote:

@KyleFromKitware oups, this fell under the cracks. I think I'm happy with is as-is, do you need help landing it?

Yes, please.

Sure. Let me know what name and email address you would like used for the patch attribution! Thanks

[KyleFromKitware]

In D143099#4233651, @cor3ntin wrote:

Sure. Let me know what name and email address you would like used for the patch attribution! Thanks

Kyle Edwards <[e-mail]>

Is there some way I can set this in my profile settings so that this name and address is always used?

[tahonermann]

Sorry for the delayed response. I managed to miss seeing this review request.

clang/include/clang/Lex/Lexer.h

598–599

clang/lib/Lex/Lexer.cpp

493–494

clang/unittests/Lex/LexerTest.cpp

673

I find this case interesting. I'm assuming it is intentional that an offset that corresponds to an EOL character indicates that the offset of the character following it be returned. That suggests some additional cases to test:

EXPECT_EQ(FindBeginningOfLineOffset("int func1();\n\n", 12), 13);
EXPECT_EQ(FindBeginningOfLineOffset("int func1();\n", 12), 13);  // 13? Or perhaps invalid?

My intuition is that an offset corresponding to an EOL character would result in the offset of the line containing the EOL character being returned.

[KyleFromKitware]

clang/unittests/Lex/LexerTest.cpp

673

I did my best to preserve the existing behavior of the function while fixing a corner case that was obviously wrong. Should this be fixed as well?

[cor3ntin]

clang/unittests/Lex/LexerTest.cpp

673

@KyleFromKitware Try to fix it. If it doesn't cause test failures, i think we can do it as part of this PR. It it has wider ramifications we'll see.
But I agree with Tom that logically new lines should be part of the line they appear in rather than the next.

clang/include/clang/Lex/Lexer.h

static inline char getCharAndSizeNoWarn(const char *Ptr, unsigned &Size,
     return getCharAndSizeSlowNoWarn(Ptr, Size, LangOpts);
   }
 
   /// Returns the leading whitespace for line that corresponds to the given
   /// location \p Loc.
   static StringRef getIndentationForLine(SourceLocation Loc,
                                          const SourceManager &SM);
 
+  /// Returns the pointer that points to the beginning of line that contains
+  /// the given offset, or null if the offset if invalid.
+  static const char *findBeginningOfLine(StringRef Buffer, unsigned Offset);
+
   /// Check if this is the first time we're lexing the input file.
   bool isFirstTimeLexingFile() const { return IsFirstTimeLexingFile; }
 
 private:
   //===--------------------------------------------------------------------===//
   // Internal implementation interfaces.
 
   /// LexTokenInternal - Internal interface to lex a preprocessing token. Called

clang/lib/Lex/Lexer.cpp

bool Lexer::getRawToken(SourceLocation Loc, Token &Result,
   // Create a lexer starting at the beginning of this token.
   Lexer TheLexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts,
                  Buffer.begin(), StrData, Buffer.end());
   TheLexer.SetCommentRetentionState(true);
   TheLexer.LexFromRawLexer(Result);
   return false;
 }
 
 /// Returns the pointer that points to the beginning of line that contains
 /// the given offset, or null if the offset if invalid.
-static const char *findBeginningOfLine(StringRef Buffer, unsigned Offset) {
+const char *Lexer::findBeginningOfLine(StringRef Buffer, unsigned Offset) {
   const char *BufStart = Buffer.data();
   if (Offset >= Buffer.size())
     return nullptr;
 
   const char *LexStart = BufStart + Offset;
-  for (; LexStart != BufStart; --LexStart) {
+  while (true) {
     if (isVerticalWhitespace(LexStart[0]) &&
         !Lexer::isNewLineEscaped(BufStart, LexStart)) {
       // LexStart should point at first character of logical line.
       ++LexStart;
       break;
     }
+    if (LexStart == BufStart)
+      break;
+    --LexStart;
   }
   return LexStart;
 }
 
 static SourceLocation getBeginningOfFileToken(SourceLocation Loc,
                                               const SourceManager &SM,
                                               const LangOptions &LangOpts) {
   assert(Loc.isFileID());
   std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
   if (LocInfo.first.isInvalid())
     return Loc;
 
   bool Invalid = false;
   StringRef Buffer = SM.getBufferData(LocInfo.first, &Invalid);
   if (Invalid)
     return Loc;
 
   // Back up from the current location until we hit the beginning of a line
   // (or the buffer). We'll relex from that point.
   const char *StrData = Buffer.data() + LocInfo.second;
-  const char *LexStart = findBeginningOfLine(Buffer, LocInfo.second);
+  const char *LexStart = Lexer::findBeginningOfLine(Buffer, LocInfo.second);
   if (!LexStart || LexStart == StrData)
     return Loc;
 
   // Create a lexer starting at the beginning of this token.
   SourceLocation LexerStartLoc = Loc.getLocWithOffset(-LocInfo.second);
   Lexer TheLexer(LexerStartLoc, LangOpts, Buffer.data(), LexStart,
                  Buffer.end());
   TheLexer.SetCommentRetentionState(true);

clang/unittests/Lex/LexerTest.cpp

TEST_F(LexerTest, RawAndNormalLexSameForLineComments) {
   EXPECT_FALSE(ToksView.empty());
   while (!L.LexFromRawLexer(T)) {
     ASSERT_TRUE(!ToksView.empty());
     EXPECT_EQ(T.getKind(), ToksView.front().getKind());
     ToksView = ToksView.drop_front();
   }
   EXPECT_TRUE(ToksView.empty());
 }
+
+TEST_F(LexerTest, FindBeginningOfLine) {
+  auto FindBeginningOfLineOffset = [](StringRef Buffer,
+                                      unsigned int Offset) -> int {
+    return Lexer::findBeginningOfLine(Buffer, Offset) - Buffer.data();
+  };
+
+  EXPECT_EQ(FindBeginningOfLineOffset("int func();", 3), 0);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func();", 0), 0);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func1();\nint func2();", 13), 13);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func1();\nint func2();", 12), 13);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func1();\nint func2();", 11), 0);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func1();\\\nint func2();", 14), 0);
+  EXPECT_EQ(FindBeginningOfLineOffset("int func1();\r\nint func2();", 13), 14);
+  EXPECT_EQ(FindBeginningOfLineOffset("\nint func();", 4), 1);
+  EXPECT_EQ(FindBeginningOfLineOffset("\\\nint func();", 5), 0);
+}
 } // anonymous namespace