[PATCH] Frontend: Fix SourceColumnMap assertion failure on non-ascii characters.
Richard Smith
richard at metafoo.co.uk
Mon Jan 5 07:18:15 PST 2015
LGTM, thank you! Sorry for the delay.
================
Comment at: test/Frontend/source-col-map.c:5-16
@@ +4,14 @@
+
+// There are two problems to be tested:
+//
+// 1. The assertion in startOfNextColumn() and startOfPreviousColumn()
+// should not be raised when the byte index is greater than the column index
+// since the non-ascii characters may use more than one bytes to store a
+// character in a column.
+//
+// 2. The length of the caret line should be equal to the number of columns of
+// source line, instead of the length of the source line. Otherwise, the
+// assertion in selectInterestingSourceRegion will be raised because the
+// removed columns plus the kept columns are not greater than the max column,
+// which means that we should not remove any column at all.
+
----------------
The comment here should explain what we're testing, not talk about the structure of the code under test -- that code might change, but this comment should not need updating when it does. This is an excellent writeup for the commit message, though.
http://reviews.llvm.org/D6746
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list