Skip to content

Conversation

@staticfloat
Copy link
Member

This abstract_call_known recursion inserted EFFECTS_TOTAL which is incorrect because <: and >: can throw if the arguments are not types.

This `abstract_call_known` recursion inserted `EFFECTS_TOTAL` which is
incorrect because `<:` and `>:` can throw if the arguments are not
types.
@staticfloat staticfloat requested a review from Keno February 6, 2023 23:06
@brenhinkeller brenhinkeller added the compiler:effects effect analysis label Feb 7, 2023
@oscardssmith oscardssmith added bugfix This change fixes an existing bug backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 labels Feb 7, 2023
@oscardssmith oscardssmith merged commit 0ab6e2f into master Feb 7, 2023
@oscardssmith oscardssmith deleted the sf/supertype_nothrow branch February 7, 2023 04:15
@aviatesk
Copy link
Member

aviatesk commented Feb 7, 2023

We can add this test case:

julia> @test !Core.Compiler.is_nothrow(Base.infer_effects((A,B)->A>:B, (Any,Any)))
Test Failed at REPL[10]:1
  Expression: !(Core.Compiler.is_nothrow(Base.infer_effects(((A, B)->begin
                    A >: B
                end), (Any, Any))))
ERROR: There was an error during testing

KristofferC pushed a commit that referenced this pull request Feb 7, 2023
This `abstract_call_known` recursion inserted `EFFECTS_TOTAL` which is
incorrect because `<:` and `>:` can throw if the arguments are not
types.

(cherry picked from commit 0ab6e2f)
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Feb 7, 2023
aviatesk added a commit that referenced this pull request Feb 8, 2023
aviatesk added a commit that referenced this pull request Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.8 Change should be backported to release-1.8 bugfix This change fixes an existing bug compiler:effects effect analysis

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants