[PATCH] Add support for Unicode in Fix-its hints

Jordan Rose jordan_rose at apple.com
Thu Jun 6 09:48:02 PDT 2013


Thanks for working on this. I'm surprised at how simple it turned out to be; good job! A few comments:

+    // Fix-its can contain Unicode characters
+    FixItEnd
+      = llvm::sys::locale::columnWidth(FixItInsertionLine.substr(0, FixItEnd));

Although FixItEnd is only used in a column context after this line, please make a separate variable anyway (since it's a byte offset up until now). Also, maybe comment on why it's safe to use the byte offset FixItStart as a column offset below.

Please add more tests as well. Tests where there are multiple fix-its on the same line seem like they'd be really important, though I can't think of any off-hand that involve identifiers. You should also include at least one test to show that the fix-it still lines up properly with the replacement range.

Finally, please put your tests in the existing test/FixIt/fixit-unicode.c. Yes, your name makes more sense. :-)

Jordan



On Jun 6, 2013, at 6:20 , Sukolsak Sakshuwong <sukolsak at gmail.com> wrote:

> This patch makes it possible to use Unicode characters in Fix-it hints.
> 
> Cheers,
> Sukolsak
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unicode.patch
Type: application/octet-stream
Size: 3246 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130606/f0c43538/attachment.obj>
-------------- next part --------------
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list