This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] utf-16 and utf-32 support in C and C++
- From: Tom Tromey <tromey at redhat dot com>
- To: Kris Van Hees <kris dot van dot hees at oracle dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 14 Mar 2008 18:09:24 -0600
- Subject: Re: [PATCH] utf-16 and utf-32 support in C and C++
- References: <20080313193208.GE19427@oracle.com>
- Reply-to: tromey at redhat dot com
>>>>> "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