-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix IRTools on Julia 1.6 #83
Conversation
terminator = true | ||
elseif isexpr(ex, GotoNode, :return) | ||
elseif ex isa GotoNode || isreturn(ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be &&
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I am pretty sure this is correct: https://github.com/FluxML/MacroTools.jl/blob/a03742057f388899833c9000ebf56dbd9616a6d5/src/utils.jl#L39. ex
can only be either a GotoNode
or a ReturnNode
, not both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
Nightly tests pass! 🎉 Could anyone merge this? |
Thanks, this is great! I will run this with Zygote and Flux on 1.6, and get on it |
FYI, this isn't enough for Zygote yet. I have started working on that in this branch and a corresponding one for ZygoteRules, but I don't currently have time to further look into what it would take to get all Zygote tests to pass on 1.6 |
Ah, alright, for now let's merge this, thanks again! |
Would it be possible to also tag a new release? |
Still needs JuliaLang/julia#38519. This passes all tests for me locally, but coverage is not great, so there might still be cases I missed.