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

Merge ??? from trunk #95

Closed
wants to merge 1 commit into from
Closed

Merge ??? from trunk #95

wants to merge 1 commit into from

Conversation

dcsobral
Copy link
Contributor

Added ??? and NotImplementedError because this seemed to be the
opinion of the majority on the "Adding ??? to Predef?" thread on
scala-internals. No review.

This change is pretty harmless, and if it's good enough for 2.10.0, it ought to be good enough for 2.9.2.

Added ??? and NotImplementedError because this seemed to be the
opinion of the majority on the "Adding ??? to Predef?" thread on
scala-internals. No review.
@ijuma
Copy link
Contributor

ijuma commented Jan 12, 2012

I don't think we should add API to the 2.9.x series without a very good reason. Otherwise people can't develop against 2.9.2 while cross-compiling to older 2.9.x series.

@dcsobral
Copy link
Contributor Author

I see your point, but adding new features in minor releases (which is what Scala's point releases really are) is common practice. Furthermore, only people cross-compiling exclusively across 2.9.x would benefit from not adding it, because as soon as you go to 2.8.x, you loose even backward compatibility. Finally, this is not a method that introduces an essential feature -- much on the contrary, in fact. It can be easily replaced if necessary, and it's main usefulness is aesthetics, teaching material and power point slides.

* `Predef.???`.
*/
final class NotImplementedError(msg: String) extends Error(msg) {
def this() = this("an implementation is missing")
Copy link
Contributor

Choose a reason for hiding this comment

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

You could get the current stack trace and figure out what method is missing implementation

Copy link
Member

Choose a reason for hiding this comment

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

+1

@ijuma
Copy link
Contributor

ijuma commented Jan 12, 2012

"I see your point, but adding new features in minor releases (which is what Scala's point releases really are) is common practice."

The closest example I can find is Java update releases and no new API is allowed on those (note that this is different from new features, Java update releases can get new HotSpot features for example).

axel22 pushed a commit to axel22/scala-github that referenced this pull request Jan 12, 2012
TreeMakers (esp. CondTreeMakers) are approximated by hash-cons'ed Conds
sharing is detected for prefixes of Conds, and shared conditions are only tested once
their results are stored, and repeated tests branch on the last shared condition,
reusing the results from the first time they were checked

a Test is 1-to-1 with a TreeMaker, but may share its Cond

TODO: clean separation of the two translation strategies:
	- naive flatMap/orElse (for virtualization)
	- less-naive if-then-else (with CSE etc coming)

sharing trees caused wrong bytecode to be emitted (verifyerror)
tentative explanation:
"because lambdalift uses mutable state to track which variables have been captured
if you refer to the same variable with the same tree twice
it'll get confused"

 Sent at 8:27 PM on Thursday
>> grzegorz.kossakowski:  so we found a bug in jvm
according to http://java.sun.com/docs/books/jvms/second_edition/html/Instructions2.doc2.html
checkcast should throw a classcastexception
becuase it's a shorthand for if !(x instanceof T) throw ClassCastExcpt
but jvm decided to throw verifyerror
and yeah, the check is wrong
if jvm was not throwing verifyerror it would throw classcast exception
saying that ObjectRef cannot be casted to $colon$colon
...
>> me:
so now where does it come from?
since a ref is involved, i thought LambdaLift
>> grzegorz.kossakowski:  yup
or now
I don't think lambalift introduces that kind of low-level casts
but I might be wrong
btw. it's interesting that it unpacks stuff from objectref twice
in your code
and in one place checkcast is correct
and in another is wrong
 Sent at 9:33 PM on Thursday
>> grzegorz.kossakowski:  also, since it's a verifyerror
I think genjvm should have an assertion

>> grzegorz.kossakowski:
  193:        getfield        scala#54; //Field scala/runtime/ObjectRef.elem:Ljava/lang/Object;
  196:        checkcast        #8; //class scala/runtime/ObjectRef
  199:        invokevirtual        scala#95; //Method scala/collection/immutable/$colon$colon.tl$1:()Lscala/collection/immutable/List;
it's this
see
you have checkcast for ObjectRef
and then on that value, you try to call tl() method from List

 Sent at 9:56 PM on Thursday
>> me:  fixed
sharing trees is bad
very bad
because lambdalift uses mutable state to track which variables have been captured
if you refer to the same variable with the same tree twice
it'll get confused
@odersky
Copy link
Contributor

odersky commented Jan 16, 2012

The policy so far is that we do want to allow new features in point releases. The difference to major releases is that we guarantee binary compatibility.

@ijuma
Copy link
Contributor

ijuma commented Jan 16, 2012

Good to know.

@paulp
Copy link
Contributor

paulp commented Jan 17, 2012

All kinds of factors weigh against adding non-essentials to 2.9.1. Let's aim at getting a 2.10 milestone out instead.

@paulp paulp closed this Jan 17, 2012
@dcsobral
Copy link
Contributor Author

I'm concentrating mainly on documentation. It's my understanding that 2.9.2 will be out before 2.10 -- if that's not true, then these pull requests are much less meaningful. But if 2.9.2 will be out first, then my reasons are:

  • When it comes to documentation, it is impossible to recover any time lost. Whatever damage is inflicted by bad documentation will stay with us.
  • ??? is useful as a teaching resource and for slideshow presentations, which is why I opened this pull request as well as the others more directly related to documentation.

szeiger pushed a commit to szeiger/scala that referenced this pull request Mar 20, 2018
da-liii pushed a commit to da-liii/scala that referenced this pull request Nov 11, 2018
Kf5-kdoctools: use Formulary.factory instead of Formula.Factory
eed3si9n pushed a commit to eed3si9n/scala that referenced this pull request May 14, 2019
lrytz pushed a commit to lrytz/scala that referenced this pull request Nov 5, 2019
retronym referenced this pull request in retronym/scala Mar 24, 2020
Fix regression around await of non-class type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants