[PATCH] utf-16 and utf-32 support in C and C++
Tom Tromey
tromey@redhat.com
Sat Mar 15 02:57:00 GMT 2008
>>>>> "Kris" == Kris Van Hees <kris.van.hees@oracle.com> writes:
Kris> * include/cpp-id-data.h (UC): Was U, conflicts with U"..."
Kris> literal.
This renaming is fine and if you want to commit it as a separate
patch, that would be ok. But, if you'd rather just keep it all as one
big patch, that is also ok by me.
I haven't read the spec. What does it say about surrogate characters?
The code seems to just defer to whatever iconv does.
I also have a few stylistic nits, nothing serious.
Kris> + switch (type) {
Brace on a new line. There are a couple instances of this with
'switch'.
Kris> tree
Kris> fix_string_type (tree value)
[...]
Kris> + if (wide) {
Kris> + if (TREE_TYPE (value) == char16_array_type_node) {
Braces on new lines, lots of instances in this function.
Kris> + type = *base == 'L' ? CPP_WSTRING
Kris> + : *base == 'U' ? CPP_STRING32
Kris> + : *base == 'u' ? CPP_STRING16
Kris> + : CPP_STRING;
Multi-line expressions like need parens around the RHS of the
assignment. And they have to be indented a bit differently. I think
the coding standards explain the details.
Kris> + if (width != CPP_OPTION(pfile, char_precision))
Space before the paren.
Kris> +static struct cset_converter
Kris> +convertor_for_type (cpp_reader *pfile, enum cpp_ttype type)
Please use "converter", not "convertor". The former is already in use
in the source, and also is hugely more popular according to google.
Kris> + cpp_string *to, enum cpp_ttype type)
Extra space before 'enum'. The nit-pickiest!
Tom
More information about the Gcc-patches
mailing list