Ticket #313 (new todo)

Opened 6 years ago

Last modified 4 years ago

ignore print -0 test errors on win32

Reported by: rurban Owned by:
Priority: normal Milestone:
Component: testing Version: trunk
Severity: medium Keywords: testing
Cc: Language:
Patch status: applied Platform: win32

Description

The win32 msvcrt has a special limitation not to print -0 as -0, instead it prints 0.

openbsd seems to have the same problem. cygwin is not affected, since it uses newlib, which is similar to the glibc in this regard.

For now I fixed the failing tests, but there should be a workaround.

t\pmc\complex.t:
not ok 380 - sinh of 0-2i
# Have: 0.000000-0.909297i
# Want: -0.000000-0.909297i
not ok 381 - sinh of 0+2i
# Have: 0.000000+0.909297i
# Want: -0.000000+0.909297i

t\pmc\float.t:
not ok 23 - neg 0
#   Failed test 'neg 0'
#   at t\pmc\float.t line 509
#                   '0'
#     doesn't match '/^-0/
# '

t\op\arithmetics.t:
not ok 7 - turn a native number into its negative
#   Failed test 'turn a native number into its negative'
#   at t\op\arithmetics.t line 175.
#          got: '0
# 0
# -123.456789
# 123.456789
# 0
# 0
# -123.456789
# 123.456789
# '
#     expected: '-0
# 0
# -123.456789
# 123.456789
# -0
# 0
# -123.456789
# 123.456789
# '

The internal numeric representation seems not to be affected. In detail, this patch does not help. It's just the printer.

Index: parrot-svn/src/ops/math.ops
===================================================================
--- parrot-svn.orig/src/ops/math.ops
+++ parrot-svn/src/ops/math.ops
@@ -774,7 +774,17 @@ inline op neg(inout INT) :base_core {
 }
 
 inline op neg(inout NUM) :base_core {
+#ifdef WIN32
+  /* The msvcrt is broken for neg -0.0 */
+  if ($1 == -0.0) {
+    $1 = -0.0;
+  }
+  else {
+    $1 = - $1;
+  }
+#else
   $1 = - $1;
+#endif
 }
 
 inline op neg(invar PMC) :base_core {
@@ -786,7 +796,17 @@ inline op neg(out INT, in INT) :base_cor
 }
 
 inline op neg(out NUM, in NUM) :base_core {
+#ifdef WIN32
+  /* The msvcrt is broken for neg -0.0 */
+  if ($2 == -0.0) {
+    $1 = -0.0;
+  }
+  else {
+    $1 = - $1;
+  }
+#else
   $1 = - $2;
+#endif
 }
 
 inline op neg(out PMC, invar PMC) :base_core { 

Attachments

tt313-win32-neg0.patch Download (2.3 KB) - added by rurban 6 years ago.
tt313-add-neg_0.patch Download (18.1 KB) - added by rurban 6 years ago.

Change History

Changed 6 years ago by rurban

Changed 6 years ago by rurban

  • status changed from new to closed
  • resolution set to fixed
  • patch changed from new to applied

Fixed by r36574.

See e.g. for exact the same buildbot failures  http://smolder.plusthree.com/app/public_projects/report_details/17922

Changed 6 years ago by chromatic

  • status changed from closed to reopened
  • resolution fixed deleted

No, you didn't "fix" these. A real fix would make the feature work, and the tests pass. That's why we have tests -- to give us confidence that features work.

Skipping failing tests that should pass is never acceptable. If you can't make them pass, an acceptable alternative is marking the tests as TODO.

Changed 6 years ago by rurban

  • patch changed from applied to rejected

changed by chromatic from skip to todo with r36575.

Marked -0.0 tests as TODO instead of failing on MSWin32 and OpenBSD, per comments on TT #313. This reverts and supersedes r36574. review: https://trac.parrot.org/parrot/changeset/36575/

This ticket will mark the openbsd and win32 workarounds to fix the printout. I'll also seperate this case in the tests.

Changed 6 years ago by rurban

  • patch changed from rejected to new

As discussed on irc the new feature has_negative_zero is checked in auto::neg_0 and used in io.ops for the -0.0 special case.

See tt313-add-neg_0.patch

Tested with has_negative_zero=1 on cygwin, debian-gcc and solaris-64-cc. But on (has_negative_zero=0) mingw, msvc6 there's more work to be done.

Looks like x == -0.0 compare fails in this libc also.

Changed 6 years ago by rurban

Changed 6 years ago by rurban

See  http://gcc.gnu.org/ml/gcc/2007-06/msg00196.html

Current problem:

0 == -0.0 => 1 on mingw/msvc

Changed 6 years ago by doughera

Correct. You should find 0.0 == -0.0 on any reasonable compiler. (Explicit testing on icc would be useful!) If it's available, the best test to use is signbit(). This has come up many times before. There is much more background in the RT tickets [RT #60312] (which includes a useful starting point for a patch) [RT #28170] and [RT #30737] (incorrectly marked as rejected, but that's ok since it's duplicated by 60312).

Changed 6 years ago by rurban

  • status changed from reopened to closed
  • resolution set to fixed

Fixed by r36638.

- add has_negative_zero feature, - fix print_n and say_n for the platforms failing the new has_negative_zero

configure test

- enable the failing tests again

This print -0.0 is a long-standing problem and finally fixes [perl #28170], [perl #30737], [perl #19183], [perl #60312].

Note that special yet untested platforms, like arm might also fail the "has_negative_zero" test and might fail the signbit workaround we use. long double -0.0 will also need a workaround code like this. There's a patch to generate the signbit() fallback at RT #30737.

Many thanks to Andy Dougherty for pointing me to the old tickets and for the solution which was there for 5 years.

M MANIFEST A config/auto/neg_0.pm M config/gen/config_h/feature_h.in M lib/Parrot/Configure/Step/List.pm M t/op/arithmetics.t M t/pmc/complex.t M t/pmc/float.t A t/steps/auto_neg_0-01.t

Changed 6 years ago by doughera

I still don't see how this is "fixed". I don't see any changes to print_n and say_n. (Nor do I understand why you'd try to fix it there rather than in src/spf_render.c). I see a new symbol PARROT_HAS_NEGATIVE_ZERO, but I don't see any code anywhere that does anything with it.

I am still confused.

Changed 6 years ago by doughera

  • status changed from closed to reopened
  • resolution fixed deleted

Changed 6 years ago by rurban

  • patch changed from new to applied

Oops, thanks. Added with r36660.

Which cases are handled by spf_render, which are not handled in print_n and say_n? These cases need to be added to the tests.

Changed 6 years ago by jimmy

Is It fixed by r36660? It still failed to me.

Changed 6 years ago by rurban

Another msvcrt.dll problem on another machine with msvc6 only with r36632. mingw passes all tests there though.

t\op\arithmetics....NOK 29#   Failed test 'Inf/NaN - basic arith'
#   in t\op\arithmetics.t at line 846.
#          got: 'Inf
# -0
# '
#     expected: 'Inf
# NaN
# '
# Looks like you failed 1 test of 29.

Changed 6 years ago by rurban

Todo the failing pmc tests with r36681.

Need to check src/spf_render.c as they are failing there.

Changed 5 years ago by whiteknight

  • milestone 1.3 deleted

Changed 5 years ago by Infinoid

In r39864, I've just un-TODOed a test on openbsd (test #4 in t/op/arithmetics.t) which had a comment mentioning this ticket. Apparently some floating point fixes to openbsd itself have improved the state of things for that particular test.

Mark

Changed 5 years ago by jkeenan

  • component changed from none to testing

Changed 5 years ago by coke

Related RT at  http://rt.perl.org/rt3/Ticket/Display.html?id=37993 (That ticket is closed in favor of this ticket.)

Changed 5 years ago by Util

In r42533, TODOed t/op/number.t test 131 (neg 0.0) where has_negative_zero is false.

Changed 4 years ago by coke

  • status changed from reopened to new
  • owner rurban deleted

Changed 4 years ago by tcurtis

  • keywords testing added
Note: See TracTickets for help on using tickets.