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

typeintersect: fix incorrect innervar handling under circular env #54545

Merged
merged 3 commits into from
May 28, 2024

Conversation

N5N3
Copy link
Member

@N5N3 N5N3 commented May 22, 2024

The infinite loop encountered in #54516 has been traced back to a circular bound during finish_unionall.
As we insert innervar more eagerly now, the direct jl_has_typevar could not find all circularity.
To address this, has_typevar_via_flatten_env is added to perform thorough check.
Although there is some code duplication with reachable_var, it could be improved in future refactoring.

#54516 also highlighted another free var escaping regression since v1.10. This regression is not solely
the result of incomplete checks, it is also caused by the missing final substitution of vb's bound, which has now been corrected.

At last, this PR adds an assertion of sorting complexity, which should facilitate the detection of similar issues by PkgEval.

close #54516

@N5N3 N5N3 added domain:types and dispatch Types, subtyping and method dispatch needs pkgeval Tests for all registered packages should be run with this change backport 1.11 Change should be backported to release-1.11 labels May 22, 2024
@N5N3
Copy link
Member Author

N5N3 commented May 22, 2024

@nanosoldier runtests()

@nanosoldier
Copy link
Collaborator

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@N5N3
Copy link
Member Author

N5N3 commented May 23, 2024

@nanosoldier runtests(["GeneralizedRouwenhorst", "MathProgBase", "Bonobo", "ManifoldDiff", "Groups", "BlockBandedMatrices", "CharacteristicInvFourier", "Pyehtim", "PolynomialRings", "NonsmoothOptim", "RangeEnclosures", "GLPKMathProgInterface", "Fri", "OOESAlgorithm", "LightGraphsFlows", "VarianceComponentModels", "PetroleumModels", "GuessworkQuantumSideInfo", "YasolSolver", "SchattenNorms", "LightGraphsMatching", "Chordal", "GARCH", "PWF", "BranchAndBound", "FreezeCurves", "Powersense", "OptimizationPRIMA", "OpSel", "RevealedPreferences", "OPFSampler", "TimetableSolver", "Algames", "PandaModels", "QUnfold", "NetDecOPF", "ApproxFunFourier", "DECAES", "OPFLearn", "ReservoirComputing", "SignalingDimension", "ConstraintModels", "CompEcon", "MarriageMarkets", "SparseSensors", "RHEOS", "QuasiCopula", "FLOWPanel", "ConstrainedSystems", "ControlSystemIdentification", "Yunir", "Catalyst", "SIAN", "FaultTolerantControl", "OptimizationMOI", "CollectiveSpins"])

@nanosoldier
Copy link
Collaborator

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@N5N3 N5N3 removed the needs pkgeval Tests for all registered packages should be run with this change label May 24, 2024
@N5N3 N5N3 requested a review from vtjnash May 24, 2024 09:36
@N5N3 N5N3 mentioned this pull request May 28, 2024
23 tasks
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing

@vtjnash vtjnash merged commit 92dfdca into JuliaLang:master May 28, 2024
7 of 9 checks passed
KristofferC pushed a commit that referenced this pull request May 29, 2024
…4545)

The infinite loop encountered in #54516 has been traced back to a
circular bound during `finish_unionall`.
As we insert innervar more eagerly now, the direct `jl_has_typevar`
could not find all circularity.
To address this, `has_typevar_via_flatten_env` is added to perform
thorough check.
Although there is some code duplication with `reachable_var`, it could
be improved in future refactoring.

#54516 also highlighted another free var escaping regression since
v1.10. This regression is not solely
the result of incomplete checks, it is also caused by the missing final
substitution of `vb`'s bound, which has now been corrected.

At last, this PR adds an assertion of sorting complexity, which should
facilitate the detection of similar issues by PkgEval.

close #54516

(cherry picked from commit 92dfdca)
@KristofferC KristofferC mentioned this pull request May 29, 2024
59 tasks
@N5N3 N5N3 deleted the circcheck branch May 29, 2024 12:17
DilumAluthge pushed a commit that referenced this pull request Jun 3, 2024
…4545)

The infinite loop encountered in #54516 has been traced back to a
circular bound during `finish_unionall`.
As we insert innervar more eagerly now, the direct `jl_has_typevar`
could not find all circularity.
To address this, `has_typevar_via_flatten_env` is added to perform
thorough check.
Although there is some code duplication with `reachable_var`, it could
be improved in future refactoring.

#54516 also highlighted another free var escaping regression since
v1.10. This regression is not solely
the result of incomplete checks, it is also caused by the missing final
substitution of `vb`'s bound, which has now been corrected.

At last, this PR adds an assertion of sorting complexity, which should
facilitate the detection of similar issues by PkgEval.
 
close #54516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.11 Change should be backported to release-1.11 domain:types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

infinite loop in C code when trying to add a method
3 participants