On Mon, Aug 29, 2022 at 05:15:26PM -0400, Jason Merrill wrote:
> On 8/29/22 04:15, Jakub Jelinek wrote:
> > Hi!
> >
> > The following patch introduces a new warning - -Winvalid-utf8 similarly
> > to what clang now has - to diagnose invalid UTF-8 byte sequences in
> > comments. In identifiers and in string literals it should be diagnosed
> > already but comment content hasn't been really verified.
> >
> > I'm not sure if this is enough to say P2295R6 is implemented or not.
> >
> > The problem is that in the most common case, people don't use
> > -finput-charset= option and the sources often are UTF-8, but sometimes
> > could be some ASCII compatible single byte encoding where non-ASCII
> > characters only appear in comments. So having the warning off by default
> > is IMO desirable. Now, if people use explicit -finput-charset=UTF-8,
> > perhaps we could make the warning on by default for C++23 and use pedwarn
> > instead of warning, because then the user told us explicitly that the source
> > is UTF-8. From the paper I understood one of the implementation options
> > is to claim that the implementation supports 2 encodings, UTF-8 and UTF-8
> > like encodings where invalid UTF-8 characters in comments are replaced say
> > by spaces, where the latter could be the default and the former only
> > used if -finput-charset=UTF-8 -Werror=invalid-utf8 options are used.
> >
> > Thoughts on this?
>
> That sounds good to me.
The pedwarn on -std=c++23 -finput-charset=UTF-8 or just documenting that
"conforming" UTF-8 is only -finput-charset=UTF-8 -Werror=invalid-utf8 ?
> > +static const uchar *
> > +_cpp_warn_invalid_utf8 (cpp_reader *pfile)
> > +{
> > + cpp_buffer *buffer = pfile->buffer;
> > + const uchar *cur = buffer->cur;
> > +
> > + if (cur[0] < utf8_signifier
> > + || cur[1] < utf8_continuation || cur[1] >= utf8_signifier)
> > + {
> > + cpp_warning_with_line (pfile, CPP_W_INVALID_UTF8,
> > + pfile->line_table->highest_line,
> > + CPP_BUF_COL (buffer),
> > + "invalid UTF-8 character <%x> in comment",
> > + cur[0]);
> > + return cur + 1;
> > + }
> > + else if (cur[2] < utf8_continuation || cur[2] >= utf8_signifier)
>
> Unicode table 3-7 says that the second byte is sometimes restricted to less
> than this range.
That is true and I've tried to include tests for all of those cases in the
testcase and all of them get a warning. Some of them are through:
/* Make sure the shortest possible encoding was used. */
if (c <= 0x7F && nbytes > 1) return EILSEQ;
if (c <= 0x7FF && nbytes > 2) return EILSEQ;
if (c <= 0xFFFF && nbytes > 3) return EILSEQ;
if (c <= 0x1FFFFF && nbytes > 4) return EILSEQ;
if (c <= 0x3FFFFFF && nbytes > 5) return EILSEQ;
and others are through:
/* Make sure the character is valid. */
if (c > 0x7FFFFFFF || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ;
All I had to do outside of what one_utf8_to_cppchar already implements was:
> > + if (_cpp_valid_utf8 (pfile, &pstr, buffer->rlimit, 0, NULL, &s)
> > + && s <= 0x0010FFFF)
the <= 0x0010FFFF check, because one_utf8_to_cppchar as written happily
supports up to 6 bytes long UTF-8 which can encode up to 7FFFFFFF:
00000000-0000007F 0xxxxxxx
00000080-000007FF 110xxxxx 10xxxxxx
00000800-0000FFFF 1110xxxx 10xxxxxx 10xxxxxx
00010000-001FFFFF 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx
00200000-03FFFFFF 111110xx 10xxxxxx 10xxxxxx 10xxxxxx 10xxxxxx
04000000-7FFFFFFF 1111110x 10xxxxxx 10xxxxxx 10xxxxxx 10xxxxxx 10xxxxxx
while 3-7 only talks about encoding 0..D7FF and D800..10FFFF in up to 4
bytes.
I guess I should try what happens with 0x110000 and 0x7fffffff in
identifiers and string literals.
Jakub
