Fix/18681 unreduced summon from fallback #19490
Conversation
This commit fixes the reporting issue where un-reduced summonFrom does not fallback to implicitNotFound. Before this commit, `reduceInlineMatch` discarded the information from implicit search in InlineReducer. This commit adds some changes so that the error message from implicit search failure can propagate to Inliner#typedMatchFinish to emit better error message.
| case _ => None | ||
| } | ||
| case Left(None) => recur(cases1, theLastMsg) | ||
| case Left(msg @Some(_)) => recur(cases1, msg) |
There was a problem hiding this comment.
Should it fail fast when a case results in missing implicit instance rather than continuing remaining cases?
i10416
changed the title
|
|
||
| inline def summonMissing = compiletime.summonFrom { | ||
| case m: Missing => m | ||
| } |
There was a problem hiding this comment.
I wonder what should be the error for
@annotation.implicitNotFound("there is no Missing1!")
trait Missing1
@annotation.implicitNotFound("there is no Missing2!")
trait Missing2
inline def summonMissing = compiletime.summonFrom {
case m: Missing1 => m
case m: Missing2 => m
}
val x = summonMissingWe can choose the first one, last one, concatenate the messages. Not sure what would be best. This might also mean that we loose information in which implicits are accepted here.
There was a problem hiding this comment.
In that case it might be better to do
inline def summonMissing = compiletime.summonFrom {
case m: Missing1 => m
case m: Missing2 => m
case _ => compiletime.error("no Missing1 or Missing2 found")
}There was a problem hiding this comment.
Currently I think that we should use compiletime.error over implicitNotFound. The error messages in the implicitNotFound usually assume that this is the only implicit beeing summoned. Reusing implicitNotFound might lead to error messages that are out of context.
There was a problem hiding this comment.
Actually, I also wonder which error message(s) it should show.
I choosed the last message rather than accumulating errors with the aim of minimizing diff in the dotty codebase.
There was a problem hiding this comment.
Reusing ... that are out of context
I agree with you. If we concat multiple implicitNotFounds that encourage users to import something at the same time, users could follow the instruction only to find ambiguous implicits, which could confuse users, but suggest all the possible actions they can take.
There was a problem hiding this comment.
After all, it feels reasonable to keep the current behavior, doesn't it?
Should/Could we report unexhaustiveness as error in summonFrom with an error message better than "cannot reduce summonFrom with ..." in the similar way as we warn on unexhaustive pattern matches to enums?
(e.g. This does not compile because no instance of Missing1 or Missing2 is found.)
There was a problem hiding this comment.
Reminiscent of implicitNotFound on parent classes. IIRC scala 2 appends (accumulates); there is an open dotty ticket.
There was a problem hiding this comment.
If you think it is better to keep unreduced summon from... to prevent users from getting confused, I will close this PR and address other issues. Otherwise, I will add changes to accumulate error messages from implicitsNotFound annotations.
There was a problem hiding this comment.
Should/Could we report unexhaustiveness as error in summonFrom with an error message better than "cannot reduce summonFrom with ..." in the similar way as we warn on unexhaustive pattern matches to enums?
(e.g. This does not compile because no instance of Missing1 or Missing2 is found.)
That sounds like a better solution. I assume that we will still see the summonForm in the inlining stack trace. Therefore we would have a simple error message followed by the more detailed explanation.
There was a problem hiding this comment.
Thank you for the feedback. I'll address this.
fix #18681
This commit fixes the reporting issue where un-reduced summonFrom does not fallback to implicitNotFound.
Before this commit,
reduceInlineMatchdiscarded the information from implicit search in InlineReducer.See https://github.com/lampepfl/dotty/pull/19490/files#diff-b34b7edb06dcd7e56dac607175a9519e98d79473371201ad3c750613db3a8579R207-R213
This commit adds some changes so that the error message from implicit search failure can propagate to
Inliner#typedMatchFinish to emit better error message.