On 11/27/2016 11:41 AM, Aaron Crane wrote: > Andy Lester via RT <perlbug-followup@perl.org> wrote: >>> By coding it the way it was, I made it easier to read, and hence >>> maintain, and I knew it would compile on both types of machines >> >> That was my intent, too, to make it easier to follow for the programmer by encapsulating the EBCDIC-only variables within the ifdef blocks. > > (Apologies for the delay in this response; I meant to follow up on > this ticket some weeks ago, but doing so apparently slipped my mind.) > > My own view is that the net consequence of this change is to make it > harder rather than easier to follow the logic in question. My biggest > concerns relate to code structured like this: > > #ifdef EBCDIC > if (convert_unicode) { > … > } > else > #endif > { … } > > This sort of code interleaves conditionals written in two very > different languages. Correctly construing the meaning of this code for > all paths requires a mental dance that effectively reimplements the > various translation phases of a C compiler. > > I'm also aware that we have a lot of code with the sort of structure > in question (and, indeed, some of it is even harder to follow than > this example), but I'd strongly prefer simplifying what we already > have, rather than letting these structures proliferate. I think the > best course of action would be to revert > 11327fa125f019cb46cfadd1fcec077cbcb8e41f (though I regret saying so, > because I know that you submitted this patch in a good-faith attempt > to improve Perl). > But, my understanding is that this patch silences compiler warnings that are not currently enabled, but that we are working to clean up the core so that they can be enabled, and if so, I think that trumps readability in this case. So the patch should remain, if my understanding is correct. And it's clear that what is the best readability is debatable, even though my taste agrees with Aaron in this case. I've spent quite a bit of effort to try to eliminate EBCDIC vs ASCII separate compilation paths, confining them to almost entirely utf8.h and utf8.c. The main exceptions are because in EBCDIC, the range [A-Z] includes more than 26 characters, and we have special handling to try to make that invisible to the Perl application. There are two affected files: toke.c because we can say tr/A-Z/foo/, and regcomp.c because of qr/[A-Z]/. This code is in one of those areas.Thread Previous | Thread Next