-
Notifications
You must be signed in to change notification settings - Fork 21
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
deprecate fallback to filter when withFilter is not found in for-comprehension desugaring #6455
Comments
Imported From: https://issues.scala-lang.org/browse/SI-6455?orig=1 |
@retronym said:
https://groups.google.com/forum/#!msg/scala-internals/-A0dXo3k0Lk/0CNXgzWo02QJ |
@adriaanm said: |
@adriaanm said: |
@adriaanm said: |
@adriaanm said: |
@Blaisorblade said: |
@adriaanm said: |
@adriaanm said: |
@soc said (edited on Feb 10, 2014 1:48:39 PM UTC): |
@Blaisorblade said: I think Scalac should never rewrite "foo.filter(arg)" to "foo.withFilter(arg)" — but Scalac does exactly this today, under certain conditions, in a half-hearted and undocumented way. This was designed for for-comprehensions, where it can make sense, but it also catches explicit calls to filter, since the difference is already lost when the rewrite happens (because parsing and some desugarings are fused together, as Paul Phillips complains about). To solve this without breaking code, the transformation should emit a deprecation warning in 2.11, explaining that this transformation will stop happening in 2.12; Martin and Adriaan agreed on this course of action. I am convinced that your argument has no relevance on whether this transformation should be deprecated. I do see that for lazy views, you might want to define withFilter to be the same as filter, but you can do so in your library, you don't need the compiler doing that for you. That's because we all want as much behavior as possible to be outside the compiler and in the library. And even if the transformation is kept in, then somebody should improve both the implementation (look at the error message above more closely to see why) and the documentation (this is not in the SLS). |
@soc said: could be that I'm completely missing the point ... I'm incredibly tired right now, but I re-read the ticket (and #2700) and now I'm even more confused. |
@Blaisorblade said: Adriaan: reassigning to you per your wish, since Simon is busy. |
@adriaanm said: |
@adriaanm said (edited on Feb 20, 2014 6:33:16 AM UTC): Perhaps a bit eager, but let's see what the build says. |
@adriaanm said: |
This was dropped for Scala 2 in scala/scala#5252 for 2.12.0-RC1, as discussed for instance in scala/bug#6455 and scala/bug#8339.
This was dropped for Scala 2 in scala/scala#5252 for 2.12.0-RC1, as discussed for instance in scala/bug#6455 and scala/bug#8339. Beyond this code, also remove `MaybeFilter` attachment and attending code introduced in fcea3d5. A caveat is that, unexpectedly, this code was added in 2016-08-24, after scala/scala#5252 was merged, as if the decision was reversed; but scala#1461 says: > Also, I went through tests in pending/pos and reclassified as far as possible. So I assume this just happened because the Scala 2 PR didn't remove our pending test.
See the error message below? I use filter but it's not defined: I see, it seems I'm a bad kid. But... where is the call to filter? Is the compiler really complaining on the call to withFilter (retorical question, answer: yes)?
I did not believe my eyes (I was also tired, so that was a good idea), but after a couple of minutes I realized I should trust them more than my computer.
Grepping withFilter on the compiler reveals the following wonder in Typers:
I know this was intended. It's even documented somewhere, IIRC about the desugaring of for comprehensions (where it makes total sense). But whenever I call withFilter? Also, this was introduced IIRC in 2.8.
Could we please, at the very least, deprecate this aliasing in 2.10, so that it can be removed in 2.11? Or improve the error message?
The text was updated successfully, but these errors were encountered: