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

Problem with modulo division with "use integer" #202

Closed
p5pRT opened this issue Jul 16, 1999 · 5 comments

Comments

@p5pRT
Copy link

commented Jul 16, 1999

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

Searchable as RT1008$

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jul 16, 1999

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

This comment has been minimized.

Copy link
Author

commented Oct 8, 2000

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

This comment has been minimized.

Copy link
Author

commented Oct 8, 2000

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

This comment has been minimized.

Copy link
Author

commented Oct 10, 2000

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

This comment has been minimized.

Copy link
Author

commented Oct 21, 2000

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.