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

SI-8719 pattern expander errors and warnings now go through context #3876

Closed

Conversation

xeno-by
Copy link
Member

@xeno-by xeno-by commented Jul 13, 2014

This ensures that typechecking custom unapplications in silent mode
doesn't leak uncatchable errors. Interestingly enough, the problem
only manifested itself for custom unapply methods, not for synthetic
ones generated for case classes.

This ensures that typechecking custom unapplications in silent mode
doesn't leak uncatchable errors. Interestingly enough, the problem
only manifested itself for custom unapply methods, not for synthetic
ones generated for case classes.
@xeno-by
Copy link
Member Author

xeno-by commented Jul 13, 2014

review @adriaanm

@xeno-by xeno-by added this to the 2.11.2 milestone Jul 13, 2014
@xeno-by xeno-by added the tested label Jul 13, 2014
@@ -460,7 +460,7 @@ abstract class UnCurry extends InfoTransform
case UnApply(fn, args) =>
val fn1 = transform(fn)
val args1 = fn.symbol.name match {
case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, patmat.alignPatterns(tree).expectedTypes)
case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, patmat.alignPatterns(global.typer.context, tree).expectedTypes)
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know, global.typer.context eq NoContext (which throws on error).
In any case, since you're after typer, context.issue will throw (and there is also no silent mode after typer). We should get rid of TypeError in my opinion, and just use reporter.error when you're not type checking. I assume the original distinction was that, after type checking, type errors indicate compiler bugs, so we should abort. Since that's not very user-friendly, most phases now catch that exception, log it (as reporter.error would've done) and then abort. This logic is also in the main phase loop (the phase is only advanced if !reporter.hasErrors).

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you suggesting to use the isPastTyper check in the error reporting portion of alignPatterns?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't think of anything better right now. I'll include this in my PR. Looking at it now.

@adriaanm
Copy link
Contributor

Thanks, could you please resubmit after the PR for adriaanm/scala@scala:2.11.x...report-filter-wip is merged (after 2.11.2)?

@xeno-by
Copy link
Member Author

xeno-by commented Sep 11, 2014

See #3974

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