Skip to content

Conversation

@yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Sep 7, 2020

The field should be either Symbol or Int. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

@yuyichao yuyichao force-pushed the yyc/typeinf/getfield-typename branch from fc15e71 to c7c418e Compare September 7, 2020 06:26
@martinholters
Copy link
Member

In #37293, I've just moved the check

            if !(isa(nv,Symbol) || isa(nv,Int))
                return Bottom
            end

further up which subsumes this. But #37293 is more than just a bugfix, so we probably want to merge and backport this, even if #37293 can then safely revert it.

@KristofferC KristofferC mentioned this pull request Sep 7, 2020
29 tasks
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.
@yuyichao yuyichao force-pushed the yyc/typeinf/getfield-typename branch from c7c418e to 49fb165 Compare September 8, 2020 04:48
@yuyichao
Copy link
Contributor Author

yuyichao commented Sep 8, 2020

Updated the fix to be similar to #37293

@martinholters martinholters merged commit 1378bb6 into master Sep 8, 2020
@martinholters martinholters deleted the yyc/typeinf/getfield-typename branch September 8, 2020 12:30
KristofferC pushed a commit that referenced this pull request Sep 8, 2020
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

(cherry picked from commit 1378bb6)
KristofferC pushed a commit that referenced this pull request Sep 8, 2020
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

(cherry picked from commit 1378bb6)
JeffBezanson pushed a commit that referenced this pull request Sep 9, 2020
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

(cherry picked from commit 1378bb6)
KristofferC pushed a commit that referenced this pull request Sep 10, 2020
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

(cherry picked from commit 1378bb6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:inference Type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants