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

fix a bug in circular type detection #35275

Merged
merged 1 commit into from
Mar 31, 2020
Merged

Conversation

JeffBezanson
Copy link
Sponsor Member

Fixes one of the issues in #34126

@JeffBezanson JeffBezanson added the bugfix This change fixes an existing bug label Mar 27, 2020
@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 27, 2020

Ah, right. This was introduced in #34223. I'm still in general skeptical (more so now that I see the mistake there that I missed on previous review) of this half-ways approach to switching this function over from the conservative approximation first implemented, to the more aggressive form (of actually checking the field types) that it could do.

CI failure is just a repr mistake with defining this test:

  Expression: repr(NFANode34126()) == "NFANode34126(Tuple{Nothing,NFANode34126}[])"
   Evaluated: "Main.Test78Main_core.NFANode34126(Tuple{Nothing,Main.Test78Main_core.NFANode34126}[])" == "NFANode34126(Tuple{Nothing,NFANode34126}[])"

@JeffBezanson JeffBezanson merged commit 3048517 into master Mar 31, 2020
@JeffBezanson JeffBezanson deleted the jb/circulartypefix branch March 31, 2020 20:31
oxinabox pushed a commit to oxinabox/julia that referenced this pull request Apr 8, 2020
ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020
vtjnash added a commit that referenced this pull request Jul 8, 2021
Similar to #35275, but through a supertype instead of through the parameters
Fixes #41503
Fixes #41349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants