Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recent changes to inline.h have broken VC6 build on Windows #16381

Closed
p5pRT opened this issue Jan 25, 2018 · 24 comments
Closed

Recent changes to inline.h have broken VC6 build on Windows #16381

p5pRT opened this issue Jan 25, 2018 · 24 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 25, 2018

Migrated from rt.perl.org#132766 (status was 'resolved')

Searchable as RT132766$

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2018

From @steve-m-hay

I've just discovered that the VC6 build on Windows is broken​:

cl -c -nologo -GF -W3 -I..\lib\CORE -I.\include -I. -I.. -DWIN32
-D_CONSOLE -DNO_STRICT -DPERLDLL -DPERL_CORE -O1 -MD -Zi -DNDEBUG
-DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -Fo.\mini\av.obj ..\av.c
av.c
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2146​: syntax error : missing ')' before identifier 'L'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(600) : error C2061​: syntax error : identifier
'S_variant_under_utf8_count'
..\inline.h(600) : error C2059​: syntax error : ';'
..\inline.h(600) : error C2059​: syntax error : 'type'
NMAKE : fatal error U1077​: 'cl' : return code '0x2'
Stop.

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2018

From @khwilliamson

Please check if this branch (which contains a number of irrelevant commits as well) fixes the problem
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2018

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2018

From @xenu

On Thu, 25 Jan 2018 06​:04​:00 -0800
Steve Hay (via RT) <perlbug-followup@​perl.org> wrote​:

I've just discovered that the VC6 build on Windows is broken​:

cl -c -nologo -GF -W3 -I..\lib\CORE -I.\include -I. -I.. -DWIN32
-D_CONSOLE -DNO_STRICT -DPERLDLL -DPERL_CORE -O1 -MD -Zi -DNDEBUG
-DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -Fo.\mini\av.obj ..\av.c
av.c
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2146​: syntax error : missing ')' before identifier 'L'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(600) : error C2061​: syntax error : identifier
'S_variant_under_utf8_count'
..\inline.h(600) : error C2059​: syntax error : ';'
..\inline.h(600) : error C2059​: syntax error : 'type'
NMAKE : fatal error U1077​: 'cl' : return code '0x2'
Stop.

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

Do we really want to keep supporting VC6? That compiler is 20 years old,
it doesn't work on modern Windows versions and I believe it's the last
version of MSVC without proper support of 64-bit integers.

I feel that VC6 support is a liability.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @Leont

On Fri, Jan 26, 2018 at 12​:22 AM, Tomasz Konojacki <me@​xenu.pl> wrote​:

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

Do we really want to keep supporting VC6? That compiler is 20 years old,
it doesn't work on modern Windows versions and I believe it's the last
version of MSVC without proper support of 64-bit integers.

I feel that VC6 support is a liability.

The question is really​: do we want to keep supporting C89 (long long
is a C99ism).

Leon

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @xenu

On Fri, 26 Jan 2018 03​:09​:33 +0100
Leon Timmermans <fawaka@​gmail.com> wrote​:

On Fri, Jan 26, 2018 at 12​:22 AM, Tomasz Konojacki <me@​xenu.pl> wrote​:

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

Do we really want to keep supporting VC6? That compiler is 20 years old,
it doesn't work on modern Windows versions and I believe it's the last
version of MSVC without proper support of 64-bit integers.

I feel that VC6 support is a liability.

The question is really​: do we want to keep supporting C89 (long long
is a C99ism).

Leon

Yeah, but it's an *extremely* common extension found in many non-C99
compilers (including the later versions of Visual C++).

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @khwilliamson

On 01/25/2018 07​:30 PM, Tomasz Konojacki wrote​:

On Fri, 26 Jan 2018 03​:09​:33 +0100
Leon Timmermans <fawaka@​gmail.com> wrote​:

On Fri, Jan 26, 2018 at 12​:22 AM, Tomasz Konojacki <me@​xenu.pl> wrote​:

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

Do we really want to keep supporting VC6? That compiler is 20 years old,
it doesn't work on modern Windows versions and I believe it's the last
version of MSVC without proper support of 64-bit integers.

I feel that VC6 support is a liability.

The question is really​: do we want to keep supporting C89 (long long
is a C99ism).

Leon

Yeah, but it's an *extremely* common extension found in many non-C99
compilers (including the later versions of Visual C++).

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And that decision will take a while to make, and there must be some
deprecation cycles even if we decide to do it, and so this bug needs to
be fixed now.

If someone wants to propose requiring C99, it should be brought up on a
different thread. My guess is that since we're still coping with the
fallout of requiring C89, it will not get a favorable consensus for the
time being.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @steve-m-hay

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl line 518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @steve-m-hay

On 25 January 2018 at 23​:22, Tomasz Konojacki <me@​xenu.pl> wrote​:

On Thu, 25 Jan 2018 06​:04​:00 -0800
Steve Hay (via RT) <perlbug-followup@​perl.org> wrote​:

I've just discovered that the VC6 build on Windows is broken​:

cl -c -nologo -GF -W3 -I..\lib\CORE -I.\include -I. -I.. -DWIN32
-D_CONSOLE -DNO_STRICT -DPERLDLL -DPERL_CORE -O1 -MD -Zi -DNDEBUG
-DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -Fo.\mini\av.obj ..\av.c
av.c
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2146​: syntax error : missing ')' before identifier 'L'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(551) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(552) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(553) : error C2059​: syntax error : 'bad suffix on number'
..\inline.h(600) : error C2061​: syntax error : identifier
'S_variant_under_utf8_count'
..\inline.h(600) : error C2059​: syntax error : ';'
..\inline.h(600) : error C2059​: syntax error : 'type'
NMAKE : fatal error U1077​: 'cl' : return code '0x2'
Stop.

It seems that VC6 doesn't understand the "ULL" suffixes on numbers in
the chunk of new code in inline.h added by commit
1d2af57 ("Avoid some branches").

Do we really want to keep supporting VC6? That compiler is 20 years old,
it doesn't work on modern Windows versions and I believe it's the last
version of MSVC without proper support of 64-bit integers.

I feel that VC6 support is a liability.

I personally agree, and have suggested dropping support before, but
the idea didn't get agreement then, and I don't think much has changed
since then to change things​:

https://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207597.html

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @steve-m-hay

On 26 January 2018 at 08​:50, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl line 518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

With current blead but commit 1d2af57
reverted, problem 1) still occurs anyway (on line 523), but I don't
see problem 2) now.

So smoke-me/khw-variant fixes the inline.h problem, but introduces
some new problem with Encode. I tried the branch with VC++ 2017 too
and that works fine, so the Encode problem is probably VC6-specific
again.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @craigberry

On Fri, Jan 26, 2018 at 3​:00 AM, Steve Hay via perl5-porters
<perl5-porters@​perl.org> wrote​:

On 25 January 2018 at 23​:22, Tomasz Konojacki <me@​xenu.pl> wrote​:

I feel that VC6 support is a liability.

I personally agree, and have suggested dropping support before, but
the idea didn't get agreement then, and I don't think much has changed
since then to change things​:

https://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207597.html

It seems to me that things *have* changed since then. If we remove
VC6 today, another (almost) five years will have passed between that
discussion and the release of 5.28. It's noble to support stuff the
vendor has long-since abandoned, but it's reasonable to ask "how
long?"

I didn't reread every message in that thread, but it seems to me it
boiled down to three items​:

1.) ActiveState binaries are built with VC6 so it needs to be
supported. But this is no longer true.
2.) You need to support VC6 to support developing on Windows XP.
Really? People still running Windows XP in 2018 have far worse
problems than we can help them with by allowing them to build current
Perl releases.
3. You need VC6 to hack the Windows DLL model into an older, simpler
form that makes it easier to do certain kinds of low-level debugging.
I'm not sure I even understood that correctly, but why should *we*
support something that Microsoft has never supported and long-since
moved its toolchain in a different direction?

So I really don't see any reason to keep it, and removing it now does
not prevent people from tinkering with 5.26.x in obsolete environments
for as long as they want.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2018

From @khwilliamson

On 01/26/2018 06​:40 AM, Steve Hay via perl5-porters wrote​:

On 26 January 2018 at 08​:50, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl line 518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

With current blead but commit 1d2af57
reverted, problem 1) still occurs anyway (on line 523), but I don't
see problem 2) now.

So smoke-me/khw-variant fixes the inline.h problem, but introduces
some new problem with Encode. I tried the branch with VC++ 2017 too
and that works fine, so the Encode problem is probably VC6-specific
again.

Is it the same hardware you're running each OS version on?

I looked at the code, and I think the new warnings are bogus. blead
gives similar warnings in some places on whatever Windows version
dromedary has.

The panic I would have to look at it more depth. If it's the same
hardware, then I would expect a compiler problem.

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2018

From @steve-m-hay

On 26 January 2018 at 16​:34, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/26/2018 06​:40 AM, Steve Hay via perl5-porters wrote​:

On 26 January 2018 at 08​:50, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant
commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl line
518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

With current blead but commit 1d2af57
reverted, problem 1) still occurs anyway (on line 523), but I don't
see problem 2) now.

So smoke-me/khw-variant fixes the inline.h problem, but introduces
some new problem with Encode. I tried the branch with VC++ 2017 too
and that works fine, so the Encode problem is probably VC6-specific
again.

Is it the same hardware you're running each OS version on?

I looked at the code, and I think the new warnings are bogus. blead gives
similar warnings in some places on whatever Windows version dromedary has.

The panic I would have to look at it more depth. If it's the same hardware,
then I would expect a compiler problem.

Both compilers were running on the same machine running Windows 8.1.

I've now tried VC6 and VC14 (VC++ 2015) on a second machine (running
Windows 7) and I get the same result - VC6 build panics; VC14 is fine.

It could well be a compiler problem, as you say.

@p5pRT
Copy link
Author

p5pRT commented Jan 28, 2018

From @khwilliamson

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as I
have special cased MSVC6, so that it hopefully works.

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2018

From @steve-m-hay

On 28 January 2018 at 17​:41, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as I have
special cased MSVC6, so that it hopefully works.

Yes, that fixes the VC6 build. Thanks.

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2018

From @steve-m-hay

On 29 January 2018 at 08​:29, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 28 January 2018 at 17​:41, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as I have
special cased MSVC6, so that it hopefully works.

Yes, that fixes the VC6 build. Thanks.

I get a test failure after building it, though :-/

C​:\Dev\Git\perl\t>.\perl harness ../ext/XS-APItest/t/utf8.t
../ext/XS-APItest/t/utf8.t .. 1/? Use of code point 0xFFFFFFFF is not
allowed; the permissible max is 0x7FFFFFFF at t/utf8.t line 149.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 324.
../ext/XS-APItest/t/utf8.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
All 324 subtests passed

Test Summary Report


../ext/XS-APItest/t/utf8.t (Wstat​: 65280 Tests​: 324 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: No plan found in TAP output
Files=1, Tests=324, 0 wallclock secs ( 0.02 usr + 0.03 sys = 0.05 CPU)
Result​: FAIL

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2018

From @steve-m-hay

On 29 January 2018 at 09​:17, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 29 January 2018 at 08​:29, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 28 January 2018 at 17​:41, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as I have
special cased MSVC6, so that it hopefully works.

Yes, that fixes the VC6 build. Thanks.

I get a test failure after building it, though :-/

C​:\Dev\Git\perl\t>.\perl harness ../ext/XS-APItest/t/utf8.t
../ext/XS-APItest/t/utf8.t .. 1/? Use of code point 0xFFFFFFFF is not
allowed; the permissible max is 0x7FFFFFFF at t/utf8.t line 149.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 324.
../ext/XS-APItest/t/utf8.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
All 324 subtests passed

Test Summary Report
-------------------
../ext/XS-APItest/t/utf8.t (Wstat​: 65280 Tests​: 324 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
Files=1, Tests=324, 0 wallclock secs ( 0.02 usr + 0.03 sys = 0.05 CPU)
Result​: FAIL

(I get the same with VC14 too, so this isn't VC6-specific.)

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2018

From @khwilliamson

On 01/29/2018 07​:01 AM, Steve Hay wrote​:

On 29 January 2018 at 09​:17, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 29 January 2018 at 08​:29, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 28 January 2018 at 17​:41, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89 anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as I have
special cased MSVC6, so that it hopefully works.

Yes, that fixes the VC6 build. Thanks.

I get a test failure after building it, though :-/

C​:\Dev\Git\perl\t>.\perl harness ../ext/XS-APItest/t/utf8.t
../ext/XS-APItest/t/utf8.t .. 1/? Use of code point 0xFFFFFFFF is not
allowed; the permissible max is 0x7FFFFFFF at t/utf8.t line 149.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 324.
../ext/XS-APItest/t/utf8.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
All 324 subtests passed

Test Summary Report
-------------------
../ext/XS-APItest/t/utf8.t (Wstat​: 65280 Tests​: 324 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
Files=1, Tests=324, 0 wallclock secs ( 0.02 usr + 0.03 sys = 0.05 CPU)
Result​: FAIL

(I get the same with VC14 too, so this isn't VC6-specific.)

So this is a new failure? If so, can you bisect it (starting with
18dcbbd, which is just a few commits),
or just try reverting the final 3 commits in that branch?

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2018

From @khwilliamson

On 01/29/2018 11​:08 AM, Karl Williamson wrote​:

On 01/29/2018 07​:01 AM, Steve Hay wrote​:

On 29 January 2018 at 09​:17, Steve Hay <steve.m.hay@​googlemail.com>
wrote​:

On 29 January 2018 at 08​:29, Steve Hay <steve.m.hay@​googlemail.com>
wrote​:

On 28 January 2018 at 17​:41, Karl Williamson
<public@​khwilliamson.com> wrote​:

On 01/25/2018 09​:04 PM, Karl Williamson wrote​:

Nonetheless, until the project decides it won't support C89
anymore, we
have to fix bugs like this.

And similarly for MSVC6.

With that in mind, please test the latest smoke-me/khw-variant, as
I have
special cased MSVC6, so that it hopefully works.

Yes, that fixes the VC6 build. Thanks.

I get a test failure after building it, though :-/

C​:\Dev\Git\perl\t>.\perl harness ../ext/XS-APItest/t/utf8.t
../ext/XS-APItest/t/utf8.t .. 1/? Use of code point 0xFFFFFFFF is not
allowed; the permissible max is 0x7FFFFFFF at t/utf8.t line 149.
# Tests were run but no plan was declared and done_testing() was not
seen.
# Looks like your test exited with 255 just after 324.
../ext/XS-APItest/t/utf8.t .. Dubious, test returned 255 (wstat
65280, 0xff00)
All 324 subtests passed

Test Summary Report
-------------------
../ext/XS-APItest/t/utf8.t (Wstat​: 65280 Tests​: 324 Failed​: 0)
   Non-zero exit status​: 255
   Parse errors​: No plan found in TAP output
Files=1, Tests=324,  0 wallclock secs ( 0.02 usr +  0.03 sys =  0.05
CPU)
Result​: FAIL

(I get the same with VC14 too, so this isn't VC6-specific.)

So this is a new failure?  If so, can you bisect it (starting with
18dcbbd, which is just a few commits),
or just try reverting the final 3 commits in that branch?

Never mind, I was able to reproduce the problem on dromedary's win32,
and I see it is from an unrelated commit that happens to be in the
branch you tested, but which hasn't been smoked on windows, and isn't
ready to be committed. I didn't realize there would be a windows
problem with it, so I just pushed the VC6 patch on this convenient branch.

On dromedary, if I revert the unrelated commits, the problem goes away,
so I'll push just the related fixes to blead

@p5pRT
Copy link
Author

p5pRT commented Jan 31, 2018

From @khwilliamson

On 01/27/2018 04​:23 AM, Steve Hay wrote​:

On 26 January 2018 at 16​:34, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/26/2018 06​:40 AM, Steve Hay via perl5-porters wrote​:

On 26 January 2018 at 08​:50, Steve Hay <steve.m.hay@​googlemail.com> wrote​:

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant
commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl line
518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

With current blead but commit 1d2af57
reverted, problem 1) still occurs anyway (on line 523), but I don't
see problem 2) now.

So smoke-me/khw-variant fixes the inline.h problem, but introduces
some new problem with Encode. I tried the branch with VC++ 2017 too
and that works fine, so the Encode problem is probably VC6-specific
again.

Is it the same hardware you're running each OS version on?

I looked at the code, and I think the new warnings are bogus. blead gives
similar warnings in some places on whatever Windows version dromedary has.

The panic I would have to look at it more depth. If it's the same hardware,
then I would expect a compiler problem.

Both compilers were running on the same machine running Windows 8.1.

I've now tried VC6 and VC14 (VC++ 2015) on a second machine (running
Windows 7) and I get the same result - VC6 build panics; VC14 is fine.

It could well be a compiler problem, as you say.

I realized later that a bunch of callers could potentially call the
function that's not working on VC6, so I thought it best to confine the
workaround to within that function. So,
597ee3f changed to do that.

When you get around to trying VC6 again, you could make sure this hasn't
broken it again.

@p5pRT
Copy link
Author

p5pRT commented Jan 31, 2018

From @steve-m-hay

On 31 January 2018 at 02​:03, Karl Williamson <public@​khwilliamson.com> wrote​:

On 01/27/2018 04​:23 AM, Steve Hay wrote​:

On 26 January 2018 at 16​:34, Karl Williamson <public@​khwilliamson.com>
wrote​:

On 01/26/2018 06​:40 AM, Steve Hay via perl5-porters wrote​:

On 26 January 2018 at 08​:50, Steve Hay <steve.m.hay@​googlemail.com>
wrote​:

On 25 January 2018 at 17​:42, Karl Williamson via RT
<perlbug-followup@​perl.org> wrote​:

Please check if this branch (which contains a number of irrelevant
commits as well) fixes the problem

I assume you mean the smoke-me/khw-variant branch? I tried that and
it's better, but still has two problems​:

1) inline.h causes warnings​:

..\inline.h(629) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data
..\inline.h(733) : warning C4244​: '+=' : conversion from 'unsigned
__int64 ' to 'unsigned int ', possible loss of data

2) the build still fails, now during Encode​:

panic​: _force_out_malformed_utf8_message should be called only when
there are errors found at Makefile.PL line 136.
Unsuccessful Makefile.PL(cpan/Encode)​: code=65280 at ..\make_ext.pl
line
518.
NMAKE : fatal error U1077​: '..\miniperl.exe' : return code '0x2'
Stop.

I haven't looked into whether such problems (the second in particular)
exist on blead with VC6 prior to the inline.h changes anyway. I will
check that later and get back to you.

With current blead but commit 1d2af57
reverted, problem 1) still occurs anyway (on line 523), but I don't
see problem 2) now.

So smoke-me/khw-variant fixes the inline.h problem, but introduces
some new problem with Encode. I tried the branch with VC++ 2017 too
and that works fine, so the Encode problem is probably VC6-specific
again.

Is it the same hardware you're running each OS version on?

I looked at the code, and I think the new warnings are bogus. blead
gives
similar warnings in some places on whatever Windows version dromedary
has.

The panic I would have to look at it more depth. If it's the same
hardware,
then I would expect a compiler problem.

Both compilers were running on the same machine running Windows 8.1.

I've now tried VC6 and VC14 (VC++ 2015) on a second machine (running
Windows 7) and I get the same result - VC6 build panics; VC14 is fine.

It could well be a compiler problem, as you say.

I realized later that a bunch of callers could potentially call the function
that's not working on VC6, so I thought it best to confine the workaround to
within that function. So, 597ee3f changed
to do that.

When you get around to trying VC6 again, you could make sure this hasn't
broken it again.

It's still working fine, thanks.

@p5pRT
Copy link
Author

p5pRT commented Jan 31, 2018

From @khwilliamson

OP agrees it is fixed
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Jan 31, 2018

@khwilliamson - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Jan 31, 2018
@p5pRT
Copy link
Author

p5pRT commented Jan 31, 2018

From @demerphq

On 26 January 2018 at 16​:01, Craig A. Berry <craig.a.berry@​gmail.com> wrote​:

On Fri, Jan 26, 2018 at 3​:00 AM, Steve Hay via perl5-porters
<perl5-porters@​perl.org> wrote​:

On 25 January 2018 at 23​:22, Tomasz Konojacki <me@​xenu.pl> wrote​:

I feel that VC6 support is a liability.

I personally agree, and have suggested dropping support before, but
the idea didn't get agreement then, and I don't think much has changed
since then to change things​:

https://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207597.html

It seems to me that things *have* changed since then. If we remove
VC6 today, another (almost) five years will have passed between that
discussion and the release of 5.28. It's noble to support stuff the
vendor has long-since abandoned, but it's reasonable to ask "how
long?"

I didn't reread every message in that thread, but it seems to me it
boiled down to three items​:

1.) ActiveState binaries are built with VC6 so it needs to be
supported. But this is no longer true.
2.) You need to support VC6 to support developing on Windows XP.
Really? People still running Windows XP in 2018 have far worse
problems than we can help them with by allowing them to build current
Perl releases.
3. You need VC6 to hack the Windows DLL model into an older, simpler
form that makes it easier to do certain kinds of low-level debugging.
I'm not sure I even understood that correctly, but why should *we*
support something that Microsoft has never supported and long-since
moved its toolchain in a different direction?

So I really don't see any reason to keep it, and removing it now does
not prevent people from tinkering with 5.26.x in obsolete environments
for as long as they want.

+1.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant