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

inference: add missing TypeVar handling for instanceof_tfunc #55884

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

aviatesk
Copy link
Sponsor Member

I thought these sort of problems had been addressed by d60f92c, but it seems some were missed. Specifically, t.a and t.b from t::Union could be TypeVar, and if they are passed to a subroutine or recursed without being unwrapped or rewrapped, errors like #55882 could occur.

This commit resolves the issue by calling unwraptv in the Union handling within instanceof_tfunc. I also found a similar issue inside nfields_tfunc, so that has also been fixed, and test cases have been added. While I haven't been able to make up a test case specifically for the fix in instanceof_tfunc, I have confirmed that this commit certainly fixes the issue reported in #55882.

I thought these sort of problems had been addressed by d60f92c, but it
seems some were missed. Specifically, `t.a` and `t.b` from `t::Union`
could be `TypeVar`, and if they are passed to a subroutine or recursed
without being unwrapped or rewrapped, errors like #55882
could occur.

This commit resolves the issue by calling `unwraptv` in the `Union`
handling within `instanceof_tfunc`. I also found a similar issue inside
`nfields_tfunc`, so that has also been fixed, and test cases have been
added. While I haven't been able to make up a test case specifically for
the fix in `instanceof_tfunc`, I have confirmed that this commit
certainly fixes the issue reported in #55882.

- fixes #55882
@giordano giordano added compiler:inference Type inference backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 labels Sep 26, 2024
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Sep 26, 2024
@aviatesk aviatesk merged commit 4b27a16 into master Sep 27, 2024
11 checks passed
@aviatesk aviatesk deleted the avi/55882 branch September 27, 2024 02:35
@aviatesk aviatesk removed the merge me PR is reviewed. Merge when all tests are passing label Sep 27, 2024
@aviatesk aviatesk mentioned this pull request Sep 27, 2024
46 tasks
aviatesk added a commit that referenced this pull request Sep 27, 2024
)

I thought these sort of problems had been addressed by d60f92c, but it
seems some were missed. Specifically, `t.a` and `t.b` from `t::Union`
could be `TypeVar`, and if they are passed to a subroutine or recursed
without being unwrapped or rewrapped, errors like #55882
could occur.

This commit resolves the issue by calling `unwraptv` in the `Union`
handling within `instanceof_tfunc`. I also found a similar issue inside
`nfields_tfunc`, so that has also been fixed, and test cases have been
added. While I haven't been able to make up a test case specifically for
the fix in `instanceof_tfunc`, I have confirmed that this commit
certainly fixes the issue reported in #55882.

- fixes #55882
@aviatesk aviatesk removed the backport 1.10 Change should be backported to the 1.10 release label Sep 27, 2024
KristofferC pushed a commit that referenced this pull request Sep 30, 2024
)

I thought these sort of problems had been addressed by d60f92c, but it
seems some were missed. Specifically, `t.a` and `t.b` from `t::Union`
could be `TypeVar`, and if they are passed to a subroutine or recursed
without being unwrapped or rewrapped, errors like #55882
could occur.

This commit resolves the issue by calling `unwraptv` in the `Union`
handling within `instanceof_tfunc`. I also found a similar issue inside
`nfields_tfunc`, so that has also been fixed, and test cases have been
added. While I haven't been able to make up a test case specifically for
the fix in `instanceof_tfunc`, I have confirmed that this commit
certainly fixes the issue reported in #55882.

- fixes #55882

(cherry picked from commit 4b27a16)
@KristofferC KristofferC mentioned this pull request Sep 30, 2024
39 tasks
KristofferC added a commit that referenced this pull request Oct 1, 2024
Backported PRs:
- [x] #55849 <!-- Mmap: fix grow! for non file IOs -->
- [x] #55863 <!-- Update TaskLocalRNG docstring according to #49110 -->
- [x] #54433 <!-- Root globals in toplevel exprs -->
- [x] #55828 <!-- Fix some corner cases of `isapprox` with unsigned
integers -->
- [x] #55890 <!-- Profile: fix order of fields in heapsnapshot & improve
formatting -->
- [x] #55884 <!-- inference: add missing `TypeVar` handling for
`instanceof_tfunc` -->
- [x] #55881 <!-- Install terminfo data under /usr/share/julia -->
- [x] #55909 <!-- do not intentionally suppress errors in precompile
script from being reported or failing the result -->
- [x] #55355 <!-- relocation: account for trailing path separator in
depot paths -->
- [x] #55917 <!-- fix rawbigints OOB issues -->
- [x] #55892 <!-- TOML: Avoid type-pirating `Base.TOML.Parser` -->
- [x] #55798 <!-- Broadcast binary ops involving strided triangular -->
- [x] #55919 <!-- Limit `@inbounds` to indexing in the dual-iterator
branch in `copyto_unaliased!` -->

Contains multiple commits, manual intervention needed:
- [ ] #54009 <!-- allow extensions to trigger from packages in [deps]
-->
- [ ] #55509 <!-- Fix cong implementation to be properly random and not
just cycling. -->
- [ ] #55569 <!-- Add a docs section about loading/precomp/ttfx time
tuning -->
- [ ] #55824 <!-- Replace regex package module checks with actual code
checks -->

Non-merged PRs with backport label:
- [ ] #55932 <!-- REPL: make UndefVarError aware of imported modules -->
- [ ] #55910 <!-- Prevent extensions from blocking parallel
pre-compilation -->
- [ ] #55908 <!-- add logic to prefer loading modules that are already
loaded -->
- [ ] #55886 <!-- irrationals: restrict assume effects annotations to
known types -->
- [ ] #55871 <!-- lowering: don't reverse handler order in
`(pop-handler-list ...)` -->
- [ ] #55870 <!-- fix infinite recursion in `promote_type` for
`Irrational` -->
- [ ] #55867 <!-- update `hash` doc string: `widen` not required any
more -->
- [ ] #55851 <!-- [REPL] Fix #55850 by using `safe_realpath` instead of
`abspath` in `projname` -->
- [ ] #55813 <!-- Check for conflicting `@ccallable` name before JIT
registration -->
- [ ] #55743 <!-- doc: heap snapshot viewing -->
- [ ] #55741 <!-- Change annotations to use a NamedTuple -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
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 compiler:inference Type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal compiler error: "Internal error: encountered unexpected error in runtime:"
3 participants