effects: taint :nothrow effect on unknown :static_parameter
#46791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aviatesk
force-pushed
the
avi/46771
branch
4 times, most recently
from
7cc9fbb
to
14243e3
Compare
|
@Keno is it okay for now to merge this as is, and try to improve the accuracy of this |
|
I would prefer to try to get at least some precision in at the same time, because I suspect we're relying on this bug for performance in a number of applications |
staticfloat
force-pushed
the
avi/46771
branch
from
dd6aab0
to
860f7c7
Compare
|
Will this need to be backported to 1.9 since it's a bugfix to the effect system? |
aviatesk
changed the title
:nothrow effect on unknown :static_parameter (conservatively):nothrow effect on unknown :static_parameter
Yes, I think we should do it. |
oscardssmith
added
kind:bugfix
|
@nanosoldier |
aviatesk
added a commit
that referenced
this pull request
Feb 7, 2023
This is an alternative to #46791. Will be filed as a PR to check the performance difference.
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
|
given that there aren't any regressions here, good to merge? |
|
There is one test failure on this PR, so don't merge now. |
staticfloat
force-pushed
the
avi/46771
branch
3 times, most recently
from
14dfe1b
to
248fb3c
Compare
…ervatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
|
@nanosoldier |
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
KristofferC
pushed a commit
that referenced
this pull request
Feb 20, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
KristofferC
pushed a commit
that referenced
this pull request
Feb 20, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
KristofferC
pushed a commit
that referenced
this pull request
Feb 20, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
KristofferC
pushed a commit
that referenced
this pull request
Feb 21, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
KristofferC
pushed a commit
that referenced
this pull request
Feb 21, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
KristofferC
pushed a commit
that referenced
this pull request
Feb 21, 2023
* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively)
With this commit, we taint `:nothrow` effect property correctly on
access to unknown `:static_parameter`, e.g.:
```julia
unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))
```
This commit implements a very conservative analysis, and thus there is a
room for improvement still, e.g.:
```julia
unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))
```
* inference: improve `:nothrow` modeling for `:static_parameter` (#46820)
* Fix test with free type params
* Test: Ignore ::Type{T} in detect_unbounded
These are only technically unbounded because of the existence of
ill-formed types. However, this function is supposed to be an API
sanity check and ordinary users should never have ill-formed types,
so for the purpose we want here, allow unboundedness in Type{T}.
---------
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit b5d17ea)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this commit, we taint
:nothroweffect property correctly on access to unknown:static_parameter, e.g.:This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.:
fix #46771