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
make apply_type error messages more understandable #12805
Conversation
…r than the internal name
anyone care to comment on this, or should I just merge it and let people try it out |
else if (!jl_is_datatype(args[0])) { | ||
JL_TYPECHK(instantiate_type, typector, args[0]); | ||
else if (!jl_is_datatype(args[0]) && !jl_is_typector(args[0])) { | ||
jl_type_error("Type{...} expression", (jl_value_t*)jl_type_type, args[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this could be confused with a literal Type{ }
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was actually the hardest one to write for that reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's ok for now; I can't think of anything better so far.
It's good for a PR like this to have examples of the old and new output, so it's very quick to judge whether the new output is nicer. Looks OK to me so far. |
side-comment: did you intend for Tuple types to be constructable with anything (except Vararg) inside? |
Old:
|
New:
|
The error messages for |
Vector is the name of the variable, and is therefore not part of the expression passed to apply_type. the current template that can be filled in is |
But the expected needs to be Type, not |
The expected is for parameter 1 of |
No, that's a bug. |
I don't think |
Improved version of this on the way. |
Bug part fixed? |
replaces #12805 - add error for Tuple{x} where x isn't a Type - add error for invalid Tuple and Union types constructed via a parameterized typealias - improve error messages
Don't encode on-disk representation information into types. Instead, encode into a sequential ODRIndex{N}, which can be used by generated functions to look up the on-disk representation.
the current
apply_type
error message assumes the user knows thatType{...}
expressions translate into calls to apply_type. This change is intended to make the error messages both more informative (more specific to the error) and more easily comprehended to those uninitiated with the bowels of the julia src code.