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

Error with shiftoperator << #2876

Closed
DartBot opened this issue May 2, 2012 · 8 comments
Closed

Error with shiftoperator << #2876

DartBot opened this issue May 2, 2012 · 8 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug
Milestone

Comments

@DartBot
Copy link

DartBot commented May 2, 2012

This issue was originally filed by @pjako


What steps will reproduce the problem?
void main() {
  print(1 << (-21));
}

What is the expected output? What do you see instead?
2048

got:
Unhandled exception:
Illegal argument(s): -21
 0. Function: 'Smi.shlFromInt' url: 'bootstrap_impl' line:1590 col:1
 1. Function: 'IntegerImplementation.<<' url: 'bootstrap_impl' line:1481 col:28
 2. Function: '::main' url: 'file:///C:/Users/pjako/dart/shifttest/shifttest.dart' line:3 col:11

What version of the product are you using? On what operating system?
6943
Windows

Please provide any additional information below.

@DartBot
Copy link
Author

DartBot commented May 3, 2012

This comment was originally written by @pjako


Also not working with build 7232, bug appears in dartium and the standalone VM, but it works fine if I compile it to js.

@ghost
Copy link

ghost commented May 3, 2012

It is presumed that a negative left shift should give an error, as its behavior is undefined. Assigning to Area-Language for clarification.

Note that ISO C states:
1185 If the value of the right operand is negative or ... , the behavior is undefined.


cc @floitschG.
Added Area-Language, Triaged labels.

@DartBot
Copy link
Author

DartBot commented May 3, 2012

This comment was originally written by @pjako


yes, but it seems that it works not this way in javascript (as well as in c#), both operate fine with such an operation.

@DartBot
Copy link
Author

DartBot commented May 4, 2012

This comment was originally written by @pjako


You can also try it on trydart...
http://try.dartlang.org/s/d1k-

and it works just fine

@floitschG
Copy link
Contributor

There are two (related) issues reported in this bug-report:

  1. should it be allowed to do negative shifts, and if yes,
  2. what should happen to bits that are shifted out. JavaScript, for example, does a cyclic shift when the shift amount is negative.

Cyclic shifts don't make sense when integers are of infinite precision. So, if Dart allowed negative shifts, it would be equivalent to shifting to the other direction with the negated amount. Example: 1 << -3 would be equivalent to 1 >> 3.

This means that the expected result of this feature request, (1 << -21 => 2048) is out.
I'm tending towards closing this feature request as "working as intended", but, for now, I will leave it open to discuss question 1.
Personally I think that non-negative shifts can catch errors, and that it is easy to work around it, if the users expects negative inputs:
 int shiftLeft(int x, int i) => i >= 0 ? x << i : x >> -i;

Wrt the dart-to-JS compiler, and dartboard (try.dartlang.org): the number semantics of programs that are compiled to JavaScript do not always follow the spec.
We are working on improving this (and the latest iteration of the compiler already checks for negative inputs), but in the near future numbers will unfortunately behave differently in JavaScript than they do in Dart.

@anders-sandholm
Copy link
Contributor

Added this to the M1 milestone.

@gbracha
Copy link
Contributor

gbracha commented May 8, 2012

Since the shift operators are user-defined, I see this as a library issue. Furthermore, it is not a defect, but a request for a change in behavior. As to the substance, I agree with Florian.


Removed Type-Defect, Area-Language labels.
Added Type-Enhancement, Area-Library labels.

@lrhn
Copy link
Member

lrhn commented Sep 18, 2012

Florian is correct: Negative shifts makes no sense when you have arbitrary precision integers.
In ECMAScript, you have only 32-bit integers wrt. bit operations, and the shift is normalized to a number in the range 0..31 (by using the value (shift & 0x1f)).
In Dart, we can't normalize it to anything reasonable - the corresponding operation would be doing modulo with Infinity.

And we don't want a left shift by a negative amount to be a right shift. That's an error waiting to be ignored.

All in all, I see no solution to giving a meaning to "1 << -21". We'll keep throwing an argument error for negative shifts.


Added WontFix label.

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Sep 18, 2012
@DartBot DartBot added this to the M1 milestone Sep 18, 2012
@kevmoo kevmoo added closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug and removed resolution-wont_fix labels Mar 1, 2016
dart-bot pushed a commit that referenced this issue Mar 23, 2021
New commits included:
```
> git log --format="%C(auto) %h %s" 0e657414a472e74ca5dd76ae0db50cc060251dec..255a3091fc278b04be74d246a3bec8743ef4d0b7
 255a3091 Vendor package:tar and package:chunked_stream (#2932)
 86bf8b20 Handle relative git-url-paths correctly when --directory (#2919)
 3716a681 Let `pub add` fail if extra arguments are passed (#2927)
 a03ac729 Minor cleanup to reduce risk of using path.current (#2924)
 e87b7b66 Added null check for name in UserInfo class (#2918)
 056a8c9a pub deps --json (#2896)
 53a69e27 Fix .packages entries of relative path deps when using --directory (#2916)
 d6308efc pub upgrade command shows count of discontinued packages (#2908)
 51744805 Upgrade to the null safe versions of all dependencies (#2913)
 e0d538c7 Introduce .pubignore (#2787)
 79f3a8b9 pub outdated: added clear message when no outdated packages. (#2898)
 22463872 `cache clean` (#2904)
 11e7b2ce `publish --dry-run` informs that the server might do more checks (#2883)
 b6977d50 Remove untrue assert (#2884)
 35841f8d Merge branch 'cherry_picks_for_2_12'
 0db3255b Don't fail on failed status listing (#2877)
 53e8ecca Don't allow outdated taking arguments (#2872)
 e83a1dc1 Enable asserts when testing pub (#2754)
 178f2edb Add --directory option (#2876)
 5aadb70e Don't fail on failed status listing (#2877)
 4bf8a927 Remove unused field (#2878)
 73ad5426 Don't allow outdated taking arguments (#2872)
 9a70949e Use Dart library to read and write tar files (#2817)
 2f74230c Do not recommend decativating packages (#2871)
 b1697a27 Use full error message string in CommandResolutionFailedException (#2870)
 16a6210d Upgrade `downgrade --help`: `downgrade` actually updates `pubspec.lock` (#2859)
 6e240ea9 Use cached version listings as heuristic when prefetching (#2851)
 58152f7c Allow trailing slash in PUB_HOSTED_URL (#2856)
 b1bf9a33 Handle poor package-listing responses robustly. (#2847)
 d941bd24 Fix request metadata when overriding dependencyType (#2848)
 ```

Change-Id: Id6ed5698330fc0cdb507e024eabc34f925ca9208
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192304
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Jonas Jensen <jonasfj@google.com>
dart-bot pushed a commit that referenced this issue Mar 24, 2021
This reverts commit 633d4ac.

Reason for revert: breaks dependency resolution in some cases.

Original change's description:
> Bump pub
>
> New commits included:
> ```
> > git log --format="%C(auto) %h %s" 0e657414a472e74ca5dd76ae0db50cc060251dec..255a3091fc278b04be74d246a3bec8743ef4d0b7
>  255a3091 Vendor package:tar and package:chunked_stream (#2932)
>  86bf8b20 Handle relative git-url-paths correctly when --directory (#2919)
>  3716a681 Let `pub add` fail if extra arguments are passed (#2927)
>  a03ac729 Minor cleanup to reduce risk of using path.current (#2924)
>  e87b7b66 Added null check for name in UserInfo class (#2918)
>  056a8c9a pub deps --json (#2896)
>  53a69e27 Fix .packages entries of relative path deps when using --directory (#2916)
>  d6308efc pub upgrade command shows count of discontinued packages (#2908)
>  51744805 Upgrade to the null safe versions of all dependencies (#2913)
>  e0d538c7 Introduce .pubignore (#2787)
>  79f3a8b9 pub outdated: added clear message when no outdated packages. (#2898)
>  22463872 `cache clean` (#2904)
>  11e7b2ce `publish --dry-run` informs that the server might do more checks (#2883)
>  b6977d50 Remove untrue assert (#2884)
>  35841f8d Merge branch 'cherry_picks_for_2_12'
>  0db3255b Don't fail on failed status listing (#2877)
>  53e8ecca Don't allow outdated taking arguments (#2872)
>  e83a1dc1 Enable asserts when testing pub (#2754)
>  178f2edb Add --directory option (#2876)
>  5aadb70e Don't fail on failed status listing (#2877)
>  4bf8a927 Remove unused field (#2878)
>  73ad5426 Don't allow outdated taking arguments (#2872)
>  9a70949e Use Dart library to read and write tar files (#2817)
>  2f74230c Do not recommend decativating packages (#2871)
>  b1697a27 Use full error message string in CommandResolutionFailedException (#2870)
>  16a6210d Upgrade `downgrade --help`: `downgrade` actually updates `pubspec.lock` (#2859)
>  6e240ea9 Use cached version listings as heuristic when prefetching (#2851)
>  58152f7c Allow trailing slash in PUB_HOSTED_URL (#2856)
>  b1bf9a33 Handle poor package-listing responses robustly. (#2847)
>  d941bd24 Fix request metadata when overriding dependencyType (#2848)
>  ```
>
> Change-Id: Id6ed5698330fc0cdb507e024eabc34f925ca9208
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192304
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
> Reviewed-by: Jonas Jensen <jonasfj@google.com>

TBR=sigurdm@google.com,jonasfj@google.com

Change-Id: I364a26468a36438bbbc887e613241030decef4d3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192748
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
dart-bot pushed a commit that referenced this issue May 11, 2021
New commits include:
```
$ git log --format="%C(auto) %h %s" 0e657414a472e74ca5dd76ae0db50cc060251dec..00c00e8adf9706bebe8f94483b7663c5f36f59d2
 00c00e8a Vendor tar (#2987)
 291705ca Being gradual migration to null-safety (#2988)
 c5f52a37 Fix CI (#2989)
 74040a45 Update to analyzer 1.5.0, migrate from deprecated AnalysisSession.getParsedUnit() (#2975)
 ce951d70 Fix dry-run tests for publishing  by ensuring there is a server to reject requests (#2978)
 018c9650 Update LICENSE (#2944)
 2614f15c Revert "Vendor package:tar and package:chunked_stream (#2932)" (#2940)
 12d9f457 Handle package:tar cancellations
 255a3091 Vendor package:tar and package:chunked_stream (#2932)
 86bf8b20 Handle relative git-url-paths correctly when --directory (#2919)
 3716a681 Let `pub add` fail if extra arguments are passed (#2927)
 a03ac729 Minor cleanup to reduce risk of using path.current (#2924)
 e87b7b66 Added null check for name in UserInfo class (#2918)
 056a8c9a pub deps --json (#2896)
 53a69e27 Fix .packages entries of relative path deps when using --directory (#2916)
 d6308efc pub upgrade command shows count of discontinued packages (#2908)
 51744805 Upgrade to the null safe versions of all dependencies (#2913)
 e0d538c7 Introduce .pubignore (#2787)
 79f3a8b9 pub outdated: added clear message when no outdated packages. (#2898)
 22463872 `cache clean` (#2904)
 11e7b2ce `publish --dry-run` informs that the server might do more checks (#2883)
 b6977d50 Remove untrue assert (#2884)
 35841f8d Merge branch 'cherry_picks_for_2_12'
 0db3255b Don't fail on failed status listing (#2877)
 53e8ecca Don't allow outdated taking arguments (#2872)
 e83a1dc1 Enable asserts when testing pub (#2754)
 178f2edb Add --directory option (#2876)
 5aadb70e Don't fail on failed status listing (#2877)
 4bf8a927 Remove unused field (#2878)
 73ad5426 Don't allow outdated taking arguments (#2872)
 9a70949e Use Dart library to read and write tar files (#2817)
 2f74230c Do not recommend decativating packages (#2871)
 b1697a27 Use full error message string in CommandResolutionFailedException (#2870)
 16a6210d Upgrade `downgrade --help`: `downgrade` actually updates `pubspec.lock` (#2859)
 6e240ea9 Use cached version listings as heuristic when prefetching (#2851)
 58152f7c Allow trailing slash in PUB_HOSTED_URL (#2856)
 b1bf9a33 Handle poor package-listing responses robustly. (#2847)
 d941bd24 Fix request metadata when overriding dependencyType (#2848)
```

Change-Id: Id7cc4c09e74c02a92bcafe1a9d9bab9431900540
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199040
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Jonas Jensen <jonasfj@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants