This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] utf-16 and utf-32 support in C and C++


>>>>> "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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]