Fix compiler assert fail on circular type inference error (issue #1334) #1339
Conversation
| @@ -675,7 +675,7 @@ bool expr_reference(pass_opt_t* opt, ast_t** astp) | |||
|
|
|||
| ast_t* type = ast_type(def); | |||
|
|
|||
| if(is_typecheck_error(type)) | |||
| if(is_typecheck_error(type) && (errors_get_count(opt->check.errors) > 0)) | |||
There was a problem hiding this comment.
Is it really the best thing to do? In a code like this
actor Main
new create(env: Env) =>
let x = x.create()
let y = y.create()only the error for x is reported. Wouldn't it be better to remove that line entierely?
There was a problem hiding this comment.
The reason I did this is because the approach of removing the line entirely made the error message for another badpony case a little worse: https://github.com/ponylang/ponyc/pull/1339/files#diff-6adc6b8a17f2bbfe51cbc0151f770ce5R263
However, I've updated the PR to use that approach - for now we can live with those consequences I think, and possibly try to improve that other error message in a separate PR.
SeanTAllen
added
needs discussion during sync
and removed
needs discussion during sync
labels
|
There's a release underway. Please rebase against master and verify your CHANGELOG entry is correctly appearing in the "unreleased" section. |
jemc
force-pushed
the
fix/1334-circular-type-infer
branch
from
551cb3f
to
c4c7e28
Compare
|
This has been updated based on @Praetonus' feedback and rebased to use the latest changelog section. |
|
CHANGELOG changed due to 0.7.0 release, this probably needs to be rebased against master. |
|
@jemc can you rebase one more time? @Praetonus ready to sign off on this? |
|
Yes, looks good to me. |
jemc
force-pushed
the
fix/1334-circular-type-infer
branch
from
c4c7e28
to
05f92fd
Compare
|
It's now rebased, so I'm merging. However, I removed the changelog entry from the PR because I'm going to use this as a guinea pig for the automated changelog process. |
jemc
force-pushed
the
fix/1334-circular-type-infer
branch
from
05f92fd
to
5c27ca8
Compare
jemc
changed the title
jemc
added
the
changelog - fixed
This PR fixes a compiler assert fail on circular type inference error.
Resolves #1334.