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

Don't discard annotations in instantiate/2 #517

Merged

Conversation

erszcz
Copy link
Collaborator

@erszcz erszcz commented Mar 3, 2023

Prior to this commit we could run into the following error:

===> Uncaught error: {badkey,text}
===> Stack trace to the error location:
[{erlang,map_get,[text,#{}],[{error_info,#{module => erl_erts_errors}}]},
 {typechecker,get_record_fields,3,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,553}]},
 {typechecker,compat_ty,4,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,356}]},
 {typechecker,any_type,4,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,504}]},
 {typechecker,subtype,3,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,190}]},
 {typechecker,type_check_call,6,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,3450}]},
 {typechecker,do_type_check_expr_in,3,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,2690}]},
 {typechecker,type_check_expr_in,3,
              [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
               {line,2449}]}]

It's caused by a record being mentioned in a spec, like:

1> h(prettypr,text,1).

  text(S::string()) -> #text{s=deep_string()}

but then instantiate/2, when called on the spec, dropping the annotation from some types, including records. Later on, in get_record_fields/3, a record without an annotation is taken as a locally defined one, which is not always the case, and a crash happens due to a lookup to the local REnv which doesn't have a given record name.

Prior to this commit we could run into the following error:

    ===> Uncaught error: {badkey,text}
    ===> Stack trace to the error location:
    [{erlang,map_get,[text,#{}],[{error_info,#{module => erl_erts_errors}}]},
     {typechecker,get_record_fields,3,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,553}]},
     {typechecker,compat_ty,4,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,356}]},
     {typechecker,any_type,4,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,504}]},
     {typechecker,subtype,3,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,190}]},
     {typechecker,type_check_call,6,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,3450}]},
     {typechecker,do_type_check_expr_in,3,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,2690}]},
     {typechecker,type_check_expr_in,3,
                  [{file,"/Users/erszcz/work/erszcz/tapl-erlang/apps/fullequirec/_checkouts/gradualizer/src/typechecker.erl"},
                   {line,2449}]}]

It's caused by a record being mentioned in a spec, like:

    1> h(prettypr,text,1).

      text(S::string()) -> #text{s=deep_string()}

but then instantiate/2, when called on the spec, dropping the annotation from some types,
including records. Later on, in get_record_fields/3, a record without an annotation is taken
as a locally defined one, which is not always the case, and a crash happens due to a lookup to the
local REnv which doesn't have a given record name.
@erszcz erszcz requested a review from zuiderkwast March 3, 2023 18:48
@erszcz erszcz changed the title Don't discard annotation in instantiate/2 Don't discard annotations in instantiate/2 Mar 3, 2023
@erszcz erszcz merged commit fc90bb8 into josefs:master Mar 3, 2023
@erszcz erszcz deleted the dont-discard-annotation-in-instantiate-2 branch March 3, 2023 18:58
Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Looks good, although I don't remember what instantiate is doing (and there's no comment to remind me).

@erszcz
Copy link
Collaborator Author

erszcz commented Mar 3, 2023

Josef didn't add one, so there's no comment ;p
It's instantiating type variables, so later on constraints can be added on them. Anyways, it's not that important - the important thing for this fix is that we drop an annotation that might contain a file reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants