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-6616 Check that unsafe operations are only called on the presentation... #1583

Merged
merged 1 commit into from Nov 14, 2012

Conversation

dragos
Copy link
Contributor

@dragos dragos commented Nov 6, 2012

... compiler thread.

The method that checks the actual constraint is @elidable, expecting it to be used
for nightly builds but stripped-off in release builds. This way we don't lose any
performance, but 'fail-fast' in IDE nightlies.

This assumes that release builds will have at least -Xelide-below ASSERTION, but
this pull request does not do that.

review by @adriaanm, @paulp

…ion compiler thread.

The method that checks the actual constraint is @elidable, expecting it to be used
for nightly builds but stripped-off in release builds. This way we don't lose any
performance, but 'fail-fast' in IDE nightlies.

This assumes that release builds will have at least `-Xelide-below ASSERTION`, but
this pull request does not do that.
@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/856/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1563/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/856/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1563/

@jsuereth
Copy link
Member

jsuereth commented Nov 6, 2012

Yet another initialization tracker.... @paulp didn't you add another one of these recently in typer?

@paulp
Copy link
Contributor

paulp commented Nov 6, 2012

Yes, that is either redundant with 'isGlobalInitialized', or we should make it so. (By which I mean, if isGlobalInitialized doesn't have exactly the desired semantics for this, I am sure we can find something which satisfies all uses.)

@dragos
Copy link
Contributor Author

dragos commented Nov 7, 2012

Thanks for pointing that out. My guess is that I can use the other method, I only need to eliminate some false positives. Even if it's not as precise as my variable, it would still be ok. I'll make the changes ASAP.

@dragos
Copy link
Contributor Author

dragos commented Nov 7, 2012

..the only problem being that isGlobalInitialized is not in 2.10.x, only in 2.11. Any thoughts on that?

@jsuereth
Copy link
Member

jsuereth commented Nov 7, 2012

@paulp is there any reason not to cherry-pick isGlobalInitialzied from master -> 2.10.x?

@paulp
Copy link
Contributor

paulp commented Nov 7, 2012

No reason.

@dragos
Copy link
Contributor Author

dragos commented Nov 8, 2012

I'll close this pull request and open another one with the cherry-pick.

@dragos dragos closed this Nov 8, 2012
@dragos dragos reopened this Nov 8, 2012
@dragos
Copy link
Contributor Author

dragos commented Nov 8, 2012

The commit itself is small, but depends on a bunch of other refactorings and can't be easily backported to 2.10.x anymore. I decided to keep this pull request for 2.10.x, and open a new one for 2.11 using the new support.

@dragos
Copy link
Contributor Author

dragos commented Nov 13, 2012

So guys, WDYT?

@paulp
Copy link
Contributor

paulp commented Nov 14, 2012

Looks good to me.

adriaanm added a commit that referenced this pull request Nov 14, 2012
SI-6616 Check that unsafe operations are only called on the presentation...
@adriaanm adriaanm merged commit 4c0b9b2 into scala:2.10.x Nov 14, 2012
@@ -51,6 +56,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
import log.logreplay
debugLog("logger: " + log.getClass + " writing to " + (new java.io.File(logName)).getAbsolutePath)
debugLog("classpath: "+classPath)
Console.err.println("\n ======= CHECK THREAD ACCESS compiler build ========\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

oops -- how come all reviewers missed this? :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Clearly the reviewers in this case were a bunch of scrubs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only printing that in the presentation compiler, if that's any consolation :)

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