cor3ntin added inline comments.
================
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);
----------------
KyleFromKitware wrote:
> tahonermann wrote:
> > 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.
> 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?
@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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143099/new/
https://reviews.llvm.org/D143099
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
