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-7247, deprecate NotNull. #2244

Merged
merged 5 commits into from Mar 18, 2013
Merged

SI-7247, deprecate NotNull. #2244

merged 5 commits into from Mar 18, 2013

Conversation

paulp
Copy link
Contributor

@paulp paulp commented Mar 13, 2013

Brought the steamroller to bear on NotNull; fired up the Zamboni afterward to smooth things over.

def notNull: Type =
if (!settings.Ynotnull.value || isNotNull || phase.erasedTypes) this
else NotNullType(this)
@deprecated("This method will be removed", "2.11.0") def notNull: Type = this
Copy link
Contributor

Choose a reason for hiding this comment

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

we're in reflect.internal -- we can probably get rid of this as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be glad to dump it, but I'm always breaking the IDE and I figured a one line identity function was cheap insurance.

@adriaanm
Copy link
Contributor

Great. I almost finished cobbling together my own PR when I checked email and saw this. This LBTM (think "better", not "lobotomy") once we prune those last unused things, unless they are used?

@paulp
Copy link
Contributor Author

paulp commented Mar 13, 2013

Will rebase/remove/reopen.

It never worked and it would periodically jump out and bite
someone. Slash and burn so new plants can take root. Eliminated
NotNullType and NotNullClass, internal elements. Removed notNull
method on Type, which no longer has a purpose. Retained 'def
isNotNull' because there are types which are known by construction
not to be null (ThisType, ConstantType as long as the constant
isn't null, etc.) and that's still useful to know.
Maybe this was useful in some way; but no way I ever saw.
I have comments which tell me this is exposed in the IDE so
I left a stub.

I also removed mkCheckInit. That probably sounds like it's related
to -Xcheckinit. Guess again, guy-who-thinks-names-mean-things. It
was only used by -Xcheck-null.
Removed NotNull from tests and the parentage of AnyVal.

Removed the tests which were actually testing anything to
do with NotNull; massaged the others to forget NotNull and/or
not to name local things NotNull.
Restarted the Zamboni and collected these as well.
Author was convinced by reviewer that clinging to
isNotNull like Linus's security blanket will not
help us with landing a picture of the Great Pumpkin.
YAGNI, Charlie Brown.
This was a little trickier than the previous. I introduced
a new method 'isBottomSubClass' which is the obvious complement
to the beloved 'isNonBottomSubClass'. In eliminating the two
call sites of containsNull I might have overshot the mark a
bit when I rewrote fourthTry and thirdTryRef, but who is going
to argue with such beauty as this.
@paulp
Copy link
Contributor Author

paulp commented Mar 13, 2013

Okay @adriaanm this should now be worthy of your attention.

@adriaanm
Copy link
Contributor

I hope so.

@ghost ghost assigned adriaanm Mar 15, 2013
@paulp
Copy link
Contributor Author

paulp commented Mar 16, 2013

@adriaanm, perhaps it's time for YOU to try to be worthy of IT. Too busy making smoothies, I bet.

@adriaanm
Copy link
Contributor

LGTM

adriaanm added a commit that referenced this pull request Mar 18, 2013
SI-7247, deprecate NotNull.
@adriaanm adriaanm merged commit d2361a5 into scala:master Mar 18, 2013
@adriaanm
Copy link
Contributor

Great JIRA is "temporarily unavailable" (I wish it was permanent). Ticket still needs to be closed.

@lrytz
Copy link
Member

lrytz commented Mar 18, 2013

i started it again

@cchantep
Copy link
Member

If I properly get it, if a value whose type is currently marked with this trait is given a null, a compilation error is raised (scala version = 2.10.4-RC2, using SBT 0.13.1):

type mismatch;
[error]  found   : Null(null)
[error]  required: ANotNullType

What will replacement for such case, either for scala-lib type (e.g. Int, Float, ...) or those outside?

@adriaanm
Copy link
Contributor

NotNull only worked in trivial cases and it proved hard to extend it beyond
them in a useful way, so we'd rather not have it at all.

On Saturday, March 15, 2014, cchantep notifications@github.com wrote:

If I properly get it, if a value whose type is currently marked with this
trait is given a null, a compilation error is raised (scala version =
2.10.4-RC2, using SBT 0.13.1):

type mismatch;
[error] found : Null(null)
[error] required: ANotNullType

What will replacement for such case, either for scala-lib type (e.g. Int,
Float, ...) or those outside?

Reply to this email directly or view it on GitHubhttps://github.com//pull/2244#issuecomment-37725193
.

@cchantep
Copy link
Member

I can understand, but that's a quite radical pov.

smarter added a commit to smarter/scala that referenced this pull request Apr 28, 2015
These rules were removed in scala#2244 and scala.NotNull itself was
deprecated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants