OWC meets 5.17.5

[NormW]

Hi,
Compiling the 5.17.5 source with OpenWatcom 1.9 and see a couple of
'new' warnings not noted in 5.17.4...

> ..\regexec.c(3607): Warning! W200: 'multicall_cop' has been referenced but never assigned a value
> ..\regexec.c(3608): Warning! W200: 'newsp' has been referenced but never assigned a value

> PERL_UNUSED_VAR(multicall_cop);
> PERL_UNUSED_VAR(newsp);

A compiler foible perhaps?
Norm

[Dave Mitchell]

On Wed, Nov 14, 2012 at 07:12:14AM -0600, Reini Urban wrote:

> DaveM added those in 81ed78b2 to get rid of 'may be used uninitialized
> compiler warnings'

Actually they were added by Jesse Luehrs in 4f8dbb2da to get rid of
"set but not used" warnings.

> for dMULTICALL, set later in CHANGE_MULTICALL_WITHDEPTH and
> PUSH_MULTICALL_WITHDEPTH,
> but not set in the failing case for (newcv != last_pushed_cv ||
> PL_comppad != last_pad)
>
> clang and gcc get pleased with PERL_UNUSED_VAR, which is ((void)multicall_cop);
> Watcom is apparently better, as reading and writing is different.
>
> Better would be to set these to NULL as done in my attached patch.

Your patch removes the PERL_UNUSED_VAR's which will cause the
'set but not used' warnings to return under gcc etc.

The part of your patch that assigns null to those variables may silence
the OpenWatcom warnings, but they are unnecessary to the code logic,
because these variables are in fact never used (just assigned to
sometimes).

> Tests still fail all over, but this is blead.

patches welcome....

--
Counsellor Troi states something other than the blindingly obvious.
-- Things That Never Happen in "Star Trek" #16

[Reini Urban]

To reperat myself and as I said in the commit message:
"regexec MULTICALL do not outsmart compiler warnings

OpenWatcom 1.9 for example is not fooled by the ((void)multicall_cop);
expression.
Correctly initialize the two new variables for the failing init case."

The init case may fail (newcv != last_pushed_cv || PL_comppad != last_pad)
and that's why the compilers warn.
--
Reini Urban
http://cpanel.net/ http://www.perl-compiler.org/

[Dave Mitchell]

And I'll repeat myself.

Those variables are assigned to in some (but not all) branches,
but those assigned values are *never* used. So assigning NULL to them in
the remaining branch is purely cosmetic, to shut up spurious compiler
warnings.

Thus, the current situation is that:

* the code is logically correct, in that the variables can never be used
uninitialised;
* In order to shut up harmless warnings with gcc etc, the PERL_UNUSED_VAR
were added.

Under your proposed patch,

* the code performs two logically unnecessary initialisations each time
round the loop;
* shuts up a harmless warning with OpenWatcom;
* re-introduces a harmless warning with gcc etc

So the net effect of your patch is:

* makes no logical change to the code;
* makes the code slightly slower;
* makes it generate harmless warnings on common compilers while silencing
a harmless warning on an obscure compiler.

--
More than any other time in history, mankind faces a crossroads. One path
leads to despair and utter hopelessness. The other, to total extinction.
Let us pray we have the wisdom to choose correctly.
-- Woody Allen

[bulk88]

Dave Mitchell wrote:
> Under your proposed patch,
>
> * the code performs two logically unnecessary initialisations each time
> round the loop;

................................

>
> So the net effect of your patch is:
>
> * makes no logical change to the code;
> * makes the code slightly slower;

..............................
>

If an auto, a const assignment to it, if never read, and never &ed of,
will be removed by any decent compiler with dead code elimination. For
example, this line (and more lines with "dTHXa(NULL);" scattered
throughout the Perl Win32 layer) I wrote, the NULL assignments disappear
if anything O level other than 0/debug is used with Visual C, which
isn't exactly at the top of best C compilers on earth list.

http://perl5.git.perl.org/perl.git/blob/0af431d354c3a5b78e68b53f23008eea39a05452:/win32/win32.c#l2589

[Dave Mitchell]

On Tue, Nov 20, 2012 at 08:05:43PM -0500, bulk88 wrote:
> Dave Mitchell wrote:
> >Under your proposed patch,
> >
> >* the code performs two logically unnecessary initialisations each time
> > round the loop;
> ................................
> >
> >So the net effect of your patch is:
> >
> >* makes no logical change to the code;
> >* makes the code slightly slower;
> ..............................
> >
>
> If an auto, a const assignment to it, if never read, and never &ed
> of, will be removed by any decent compiler with dead code
> elimination.

Good point.

I've applied the part of Reini's patch that does the two NULL assignments,
but skipped the part that removed the two PERL_UNUSED_VAR's.

c31ee3bbc31a4f8c5b4850ce38fb8c353dca688f

--
Wesley Crusher gets beaten up by his classmates for being a smarmy git,
and consequently has a go at making some friends of his own age for a
change.
-- Things That Never Happen in "Star Trek" #18