Re: UCNs in identifiers

[Zack Weinberg <zack at codesourcery dot com>]

Geoffrey Keating <geoffk@geoffk.org> writes:

> Hi Joseph,
>
> I'll start with some general comments.
>
>> This doesn't look like a good-faith attempt to follow the checklist
>> I agreed with Zack in January as a compromise for how this feature
>> could be implemented in a way acceptable to go in GCC at all (bug
>> 9449, comment 21).
>
> Although I do not agree with the checklist, I have followed it where
> I saw no strong reason not to.

If you intended not to follow the checklist, you should have said so
when we were still discussing the checklist.  I understood you to have
agreed to follow the checklist despite not agreeing with every detail
of it.

Your wording implies that you see strong reasons not to follow the
checklist in some areas, so I would like to hear what those are,
ideally in further comments on bug 9449 rather than in followups to
this thread.

> Of course, there is still work to be done; I'm happy to leave that
> to you and Zack if you would like to do it.  As I said, though, I'm
> confident that this patch is an improvement on the current
> situation.

We agreed to disagree about whether or not an incomplete
implementation was worse or better than no implementation at all, but
that did not mean that your opinion would govern acceptance of a
patch.  If you want this feature to go in, you will either do it the
way that Joseph and I outlined in that checklist, or convince us that
we are wrong.

Also, I don't appreciate your attempt to push tasks off on us.  You're
the one who wants this feature, so it's your responsibility to do the
work.

> The tests are not testing particular properties of UCNs.  They are
> testing code paths; that is, they are 'white-box' tests, not
> 'black-box' tests.  This leads to a smaller and more useful
> testsuite.

The data-drivenness of the compiler in general makes code-coverage a
poor reed to lean on.

>> It looks to me like there are many missing pieces, including critical ones 
>> for having this feature available with -std=c99 / in C++ at all.  For 
>> example, the default warning for identifiers (as preprocessing tokens at 
>> any stage) not in NFKC doesn't seem to be implemented - having such a 
>> warning by default was a critical feature for having any agreement that 
>> this should be implemented at all.
>
> I will implement this in a follow-up patch.

If you want to do that, your first patch needs to arrange to have the
entire extended-identifier feature disabled by default (at configure
time or at run time, whichever is more convenient).

>> I don't see any commitment to the required due diligence audits of
>> everywhere an identifier of one kind (input, internal, assembler,
>> diagnostic) is converted into another kind; for example, all the
>> places that use %s with IDENTIFIER_POINTER which is no longer valid
>> when the internal format may not be suitable for diagnostics.  (The
>> command line is an input source, just as source files are, so the
>> same issues apply to it as apply to source files.)
>
> I'm satisfied that this area works as I intended.

Please stop trying to avoid this task.  Yes, we are in stage 1, but
increased incidence of compiler crashes is still a poor trade for a
tick-list feature.

zw