llvm dev - Oct 2011 - [LLVMdev] [cfe-dev] Unicode path handling on Windows

On Tue, Oct 4, 2011 at 4:19 AM, Nikola Smiljanic <popizdeh at gmail.com>
wrote:
> In that case I think that this is it :)
>
> On Tue, Oct 4, 2011 at 8:24 AM, Bryce Cogswell <bryceco at gmail.com>
wrote:
>>
>> That should be fine. I don't believe the concern about performing a
>> char-by-char conversion is valid; for example the NTFS-3G driver uses a
>> simplistic upcase table and seems to work fine. I suspect Windows does
the
>> same.
Index: include/llvm/Support/FileSystem.h
==================================================================---
include/llvm/Support/FileSystem.h	(revision 141071)
+++ include/llvm/Support/FileSystem.h	(working copy)
@@ -436,6 +436,32 @@

+  for (int i = 0; i != argc; ++i)
+  {
+    // check lenght

May want to correct the typo.

Index: lib/Basic/ConvertUTF.c
==================================================================---
lib/Basic/ConvertUTF.c	(revision 141071)
+++ lib/Basic/ConvertUTF.c	(working copy)
@@ -218,74 +218,7 @@

+      /* Figure out how many bytes the result will require */
+      if (ch < (UTF32)0x80) {      bytesToWrite = 1;
+      } else if (ch < (UTF32)0x800) {     bytesToWrite = 2;
+      } else if (ch < (UTF32)0x10000) {   bytesToWrite = 3;
+      } else if (ch < (UTF32)0x110000) {  bytesToWrite = 4;
+      } else {                            bytesToWrite = 3;
+      ch = UNI_REPLACEMENT_CHAR;
+      }

I think this should be formatted more like this to meet our coding
standards (but am not 100% sure, so I will defer to others):

if (ch < (UTF32)0x80)
   bytesToWrite = 1;
else if (ch < (UTF32)0x800)
  bytesToWrite = 2;
...
else
  ch = UNI_REPLACEMENT_CHAR;

Those are the only minor nitpicks, well done!

~Aaron

>
>
> +  for (int i = 0; i != argc; ++i)
> +  {
> +    // check lenght
>
> May want to correct the typo.
>
>
Fixed

>
> I think this should be formatted more like this to meet our coding
> standards (but am not 100% sure, so I will defer to others):
>
> if (ch < (UTF32)0x80)
>   bytesToWrite = 1;
> else if (ch < (UTF32)0x800)
>  bytesToWrite = 2;
> ...
> else
>  ch = UNI_REPLACEMENT_CHAR;
>
>
I haven't touched this code. I only took the function out of the section
that was commented out.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111004/84d54a66/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.patch
Type: application/octet-stream
Size: 15811 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111004/84d54a66/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm.patch
Type: application/octet-stream
Size: 4685 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111004/84d54a66/attachment-0001.obj>

ping
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111010/14a4f67c/attachment.html>