Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
2.05k stars 566 forks source link

Problem with modulo division with "use integer" #202

Closed p5pRT closed 21 years ago

p5pRT commented 25 years ago

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

Searchable as RT1008$

p5pRT commented 25 years ago

From sbeck@cise.ufl.edu

  #!/usr/local/bin/perl -w

  $d = -1 % 7;   print "$d\n";

  use integer;

  $d = -1 % 7;   print "$d\n";

In all cases\, it gives the output

  6   -1

The first result is correct.

The man page says that the operator is not as well defined for negative operands\, so perhaps this can't be treated as a bug per-se\, but it would be nice if the "use integer" didn't change the behavior of the operator.

I've included the "perl -V" info for Solaris 2.6 below\, but I doubt that it's relevant to the problem.

Summary of my perl5 (5.0 patchlevel 5 subversion 3) configuration​:   Platform​:   osname=solaris\, osvers=2.6\, archname=sun4-solaris-thread   uname='sunos cave 5.6 generic_105181-12 sun4u sparc sunw\,ultra-2 '   hint=recommended\, useposix=true\, d_sigaction=define   usethreads=define useperlio=undef d_sfio=undef   Compiler​:   cc='cc'\, optimize='-xO4'\, gccversion=   cppflags='-D_REENTRANT -I/usr/local/include'   ccflags ='-D_REENTRANT -I/usr/local/include'   stdchar='unsigned char'\, d_stdstdio=define\, usevfork=true   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   alignbytes=8\, usemymalloc=y\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags =' -L/usr/local/lib'   libpth=/usr/local/lib /lib /usr/lib /usr/ccs/lib   libs=-lsocket -lnsl -ldl -lm -lpthread -lc -lcrypt   libc=/lib/libc.so\, so=so\, useshrplib=true\, libperl=libperl.so   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='   -R /usr/local/lib/perl5/5.00503/sun4-solaris-thread/CORE'   cccdlflags='-KPIC'\, lddlflags='-G -L/usr/local/lib'

---------------------------- Sullivan Beck ----------------------------- sbeck@​cise.ufl.edu | This space reserved for some saying demonstrating CSE 314E | great wisdom\, wit\, or insight. I'll fill it in PH : (352) 392-1057 | just as soon as I have any of the above. Fax​: (352) 392-1220 | ------------------- http​://www.cise.ufl.edu/~sbeck/ --------------------

p5pRT commented 24 years ago

From @rspier

From the 'old bug patrol'....

It doesn't seem that there is a good\, fast\, and platform independent solution to the fact that C leaves certain aspects of modulo with negative numbers undefined. (Or more specifically - does allow it to be machine dependent.)

There are several bugs in the DB related to this​:   19990716.004   19991022.018   20000821.002

Here's a patch (against perl-current) which adds a warning. [below]

The warning probably isn't the best solution - but there probably isn't a 'best' one - but this at least explains what's going on to the user who didn't read the documentation.

There is also a fallback in pp_modulo\, for UV's which ends up doing   ans = left % right; Should a similar warning go there too?

-R

[rspier@​speed perl-current]$ diff -pc pp.c.orig pp.c *** pp.c.orig Sun Oct 8 19​:10​:43 2000 --- pp.c Sun Oct 8 20​:05​:55 2000 *************** PP(pp_i_modulo) *** 1525\,1530 **** --- 1525\,1533 ----   dPOPTOPiirl;   if (!right)   DIE(aTHX_ "Illegal modulus zero"); + if (ckWARN(WARN_PORTABLE) && (left \< 0 || right \< 0)) + Perl_warner(aTHX_ WARN_PORTABLE\, + "Results of %% with negative numbers and use integer is platform dependent");   SETi( left % right );   RETURN;   }

p5pRT commented 24 years ago

From @gsar

On Sun\, 08 Oct 2000 20​:14​:40 EDT\, Robert Spier wrote​:

[rspier@​speed perl-current]$ diff -pc pp.c.orig pp.c *** pp.c.orig Sun Oct 8 19​:10​:43 2000 --- pp.c Sun Oct 8 20​:05​:55 2000 *************** PP(pp_i_modulo) *** 1525\,1530 **** --- 1525\,1533 ---- dPOPTOPiirl; if (!right) DIE(aTHX_ "Illegal modulus zero"); + if (ckWARN(WARN_PORTABLE) && (left \< 0 || right \< 0))

Putting the cheaper test will be more efficient in such cases​:

  if ((left \< 0 || right \< 0) && ckWARN(WARN_PORTABLE))

+ Perl_warner(aTHX_ WARN_PORTABLE\, + "Results of %% with negative numbers and use integer is pl atform dependent"); SETi( left % right ); RETURN; }

Given that what it is warning about is what it has been documented to do for a long time\, I don't think I agree with the need for this patch.

Sarathy gsar@​ActiveState.com

p5pRT commented 24 years ago

From @rspier

GS> Putting the cheaper test will be more efficient in such cases​: GS> if ((left \< 0 || right \< 0) && ckWARN(WARN_PORTABLE))

Oops. Changed.

GS> Given that what it is warning about is what it has been documented GS> to do for a long time\, I don't think I agree with the need for GS> this patch.

I agree with that.

But also - there have been a few bug reports on this subject\, and related to it. Yes - the problem is with the C standard (or lack thereof\, in this case)\, and it seems that people expect modulo to be the same on all systems.

I think the warning is harmless - and might encourage people to "do the proper thing" and be aware of cross platform issues. perl 5.6 added many warnings\, as compared to 5.6.0.

If the warning annoys people - they should be using 'no warnings "portable";'

If your opinion is still No\, I'll close the bug and note that "It's working as documented."

-R

p5pRT commented 24 years ago

From The RT System itself

Functionality is documented to work this way. The "bug" is in the definition of the C standard for %. Closing because there seems to be little interest in fixing or adding warnings.