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

isassigned for ranges with BigInt indices #50587

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Jul 18, 2023

This fixes certain possible regressions in isassigned for ranges to restore the 1.9-like behavior.
On v1.9

julia> r = 1:big(2)^65
1:36893488147419103232

julia> isassigned(r, lastindex(r))
true

julia> isassigned(r, true)
ERROR: ArgumentError: invalid index: true of type Bool

On v1.10.0-alpha1

julia> isassigned(r, lastindex(r))
ERROR: InexactError: Int64(36893488147419103232)

julia> isassigned(r, true) # Bool is converted to Int
true

julia> r[true] # but indexing with Bool doesn't work
ERROR: ArgumentError: invalid index: true of type Bool

This PR

julia> isassigned(r, lastindex(r))
true

julia> isassigned(r, true)
ERROR: ArgumentError: invalid index: true of type Bool

This still leaves

julia> isassigned(collect(1:3), true)
true

so that should perhaps be changed as well.

@vtjnash vtjnash merged commit d270a71 into master Jul 18, 2023
2 checks passed
@vtjnash vtjnash deleted the jishnub/rangeisassigned branch July 18, 2023 19:54
@vtjnash vtjnash added the backport 1.10 Change should be backported to the 1.10 release label Jul 18, 2023
KristofferC pushed a commit that referenced this pull request Jul 24, 2023
This fixes certain possible regressions in `isassigned` for ranges to
restore the 1.9-like behavior.
On v1.9
```julia
julia> r = 1:big(2)^65
1:36893488147419103232

julia> isassigned(r, lastindex(r))
true

julia> isassigned(r, true)
ERROR: ArgumentError: invalid index: true of type Bool
```
On v1.10.0-alpha1
```julia
julia> isassigned(r, lastindex(r))
ERROR: InexactError: Int64(36893488147419103232)

julia> isassigned(r, true) # Bool is converted to Int
true

julia> r[true] # but indexing with Bool doesn't work
ERROR: ArgumentError: invalid index: true of type Bool
```
This PR
```julia
julia> isassigned(r, lastindex(r))
true

julia> isassigned(r, true)
ERROR: ArgumentError: invalid index: true of type Bool
```
This still leaves
```julia
julia> isassigned(collect(1:3), true)
true
```
so that should perhaps be changed as well.

(cherry picked from commit d270a71)
KristofferC added a commit that referenced this pull request Jul 24, 2023
Backported PRs:
- [x] #50411 <!-- Fix weird dispatch of * with zero arguments -->
- [x] #50202 <!-- Remove dynamic dispatch from _wait/wait2 -->
- [x] #50064 <!-- Fix numbered prompt with input only with comment -->
- [x] #50026 <!-- Store heapsnapshot files in tempdir() instead of
current directory -->
- [x] #50402 <!-- Add CPU feature helper function -->
- [x] #50387 <!-- update newpages pointer after actually sweeping pages
-->
- [x] #50424 <!-- avoid potential type-instability in _replace_(str,
...) -->
- [x] #50444 <!-- Optimize getfield lowering to avoid boxing in some
cases -->
- [x] #50474 <!-- docs: Fix a `!!! note` which was miscapitalized -->
- [x] #50466 <!-- relax assertion involving pg->nold to reflect that it
may be a bit in… -->
- [x] #50490 <!-- Fix compat annotation for italic printstyled -->
- [x] #50488 <!-- fix typo in `Base.isassigned` with `Tridiagonal` -->
- [x] #50476 <!-- Profile: Add specifying dir for `take_heap_snapshot`
and handling if current dir is unwritable -->
- [x] #50461 <!-- fix typo in the --gcthreads argument description -->
- [x] #50528 <!-- ssair: Correctly handle stmt insertion at end of basic
block -->
- [x] #50533 <!-- ensure internal_obj_base_ptr checks whether objects
past freelist pointer are in freelist -->
- [x] #49322 <!-- improve cat design / performance -->
- [x] #50540 <!-- gc: remove over-eager assertion -->
- [x] #50542 <!-- gf: remove unnecessary assert cycle==depth -->
- [x] #50559 <!-- Expand kwcall lowering positional default check to
vararg -->
- [x] #50058 <!-- Add unwrapping mechanism for triangular mul and solves
-->
- [x] #50551 <!-- typeintersect: also record chained `innervars` -->
- [x] #50552 <!-- read(io, Char): fix read with too many leading ones
-->
- [x] #50541 <!-- precompile: ensure globals are not accidentally
created where disallowed -->
- [x] #50576 <!-- use atomic compare exchange when setting the GC
mark-bit -->
- [x] #50578 <!-- gf: make method overwrite/delete an error during
precompile -->
- [x] #50516 <!-- Fix visibility of assert on GCC12/13 -->
- [x] #50597 <!-- Fix memory corruption if task is launched inside
finalizer -->
- [x] #50591 <!-- build: fix various makefile bugs -->
- [x] #50599 <!-- faster invalid object lookup in conservative gc -->
- [x] #50634 <!-- 🤖 [master] Bump the SparseArrays stdlib from b4b0e72
to 99c99b4 -->
- [x] #50639 <!-- Backport LLVM patches to fix various issues. -->
- [x] #50546 <!-- Revert storage of method instance in LineInfoNode -->
- [x] #50631 <!-- Shift DCE pass to optimize imaging mode code better
-->
- [x] #50525 <!-- only check that values are finite in `generic_lufact`
when `check=true` -->
- [x] #50587 <!-- isassigned for ranges with BigInt indices -->
- [x] #50144 <!-- Page based heap size heuristics -->


Need manual backport:
- [ ] #50595 <!-- Rename ENV variable `JULIA_USE_NEW_PARSER` ->
`JULIA_USE_FLISP_PARSER` -->



Non-merged PRs with backport label:
- [ ] #50637 <!-- Remove SparseArrays legacy code -->
- [ ] #50618 <!-- inference: continue const-prop' when concrete-eval
returns non-inlineable -->
- [ ] #50598 <!-- only limit types in stack traces in the REPL -->
- [ ] #50594 <!-- Disallow non-index Integer types in isassigned -->
- [ ] #50568 <!-- `Array(::AbstractRange)` should return an `Array` -->
- [ ] #50523 <!-- Avoid generic call in most cases for getproperty -->
- [ ] #50172 <!-- print feature flags used for matching pkgimage -->
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Jul 24, 2023
@brenhinkeller brenhinkeller added the domain:bignums BigInt and BigFloat label Aug 8, 2023
jishnub added a commit that referenced this pull request Aug 10, 2023
Extend #50587 to more general `AbstractArray`s. This is mainly to
disallow `Bool` as an index in `isassigned`, as this isn't supported by
`getindex`. After this
```julia
julia> isassigned(rand(2,2), 1, true)
ERROR: ArgumentError: invalid index: true of type Bool
```
which matches the behavior on v1.9.
IanButterworth pushed a commit that referenced this pull request Aug 19, 2023
Extend #50587 to more general `AbstractArray`s. This is mainly to
disallow `Bool` as an index in `isassigned`, as this isn't supported by
`getindex`. After this
```julia
julia> isassigned(rand(2,2), 1, true)
ERROR: ArgumentError: invalid index: true of type Bool
```
which matches the behavior on v1.9.

(cherry picked from commit b991397)
nalimilan pushed a commit that referenced this pull request Nov 5, 2023
Extend #50587 to more general `AbstractArray`s. This is mainly to
disallow `Bool` as an index in `isassigned`, as this isn't supported by
`getindex`. After this
```julia
julia> isassigned(rand(2,2), 1, true)
ERROR: ArgumentError: invalid index: true of type Bool
```
which matches the behavior on v1.9.

(cherry picked from commit b991397)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:bignums BigInt and BigFloat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants