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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
