[PATCH] D143099: [clang][lex] Expose findBeginningOfLine()

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

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



================
Comment at: clang/include/clang/Lex/Lexer.h:598-599
 
+  /// 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);
----------------



================
Comment at: clang/lib/Lex/Lexer.cpp:493-494
 
 /// Returns the pointer that points to the beginning of line that contains
 /// the given offset, or null if the offset if invalid.
+const char *Lexer::findBeginningOfLine(StringRef Buffer, unsigned Offset) {
----------------
KyleFromKitware wrote:
> cor3ntin wrote:
> > Should we remove the comment here?
> 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.



================
Comment at: clang/unittests/Lex/LexerTest.cpp:673
+  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);
----------------
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.


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

https://reviews.llvm.org/D143099