-
Notifications
You must be signed in to change notification settings - Fork 111
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 type check errors use the to-string() method directly #1527
base: horizon
Are you sure you want to change the base?
Conversation
It looks like constructor printing used to be overridden for the sake of errors. However, this is silly because we want constructor printing for spying on types and so on within the compiler. So make the error rendering contexts use to-string explicitly. Down the line this should be a richer renderable type struct, but ED.embed does constructor printing as well so stringifying with the existing stringifier is better.
I had commented (b5dfaa3#r41383423) about this and committed (92ec0c0) a fix, and this PR seems to take a different approach. (In any case, #1525 is effectively currently fixed on horizon, without this PR.) I don't quite understand "However, this is silly because we want constructor printing for spying on types and so on within the compiler" though -- why? I got the sense that since this method was simply commented out, that it was a straggler of a commit that didn't mean to be committed. I'd be happier with the inverse of this PR, I think: make |
If we do go with this PR, then you'll need to revert that commit, or else you won't get back your constructor printing, anyway... |
The issue with overriding |
To follow on to this discussion – I just ran into a situation where I wanted to print a |
It looks like constructor printing used to be overridden for the sake
of errors. However, this is silly because we want constructor printing
for spying on types and so on within the compiler.
So make the error rendering contexts use to-string explicitly.
Down the line this should be a richer renderable type struct, but
ED.embed does constructor printing as well so stringifying with
the existing stringifier is better.
@blerner @mkolosick can you do a brief review of this to make sure I'm not missing something obvious?