Skip to content

SI-5739 (bis) vals for subpatterns when -g:patmatvars#1060

Merged
adriaanm merged 1 commit intoscala:2.10.xfrom
adriaanm:ticket-5739b
Aug 8, 2012
Merged

SI-5739 (bis) vals for subpatterns when -g:patmatvars#1060
adriaanm merged 1 commit intoscala:2.10.xfrom
adriaanm:ticket-5739b

Conversation

@adriaanm
Copy link
Contributor

@adriaanm adriaanm commented Aug 6, 2012

To facilitate debugging pattern matches, we store the values for
sub-patterns of extractor (synthetic or user-defined) patterns when -g:patmatvars.
Normally we inline them directly unless compiling under -g:patmatvars

For soundness, SI-5158, SI-6070, we must always store the values of mutable case class fields.

We don't always do this since it may affect performance.
Also, the total cost in byte code size is about 1% for the compiler&stdlib
when run with -optimize and -g:patmatvars
(but see SI-6193: dead case class field accessor calls are not eliminated).

There were suspicions that emitting this many local variables may impact run-time performance
(it surely affects compiler performance) thus, I've decided to make these definitions opt-in.
Simply pass -g:patmatvars to scalac to facilitate debugging of pattern matches.

@jsuereth
Copy link
Contributor

jsuereth commented Aug 6, 2012

Review by?

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 6, 2012

still pondering whether we should actually do this, so no reviewer yet

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 7, 2012

I've thought about it and think we should hide this behind a flag as the deluge of local variables slows down the optimizer considerably.

review by @paulp /cc @odersky

@dotta
Copy link
Contributor

dotta commented Aug 7, 2012

This is relevant for the Scala IDE debugger, it would be good to get Luc's opinion (this was also discussed on scala-internals http://groups.google.com/group/scala-internals/browse_thread/thread/d38fdf2fcd74e038) \cc @skyluc

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 8, 2012

based on discussion during the scala meeting, predicating emission of local vals for subpats on absence of -optimize

PLS REBUILD ALL

To facilitate debugging pattern matches, we store the values for
sub-patterns of extractor (synthetic or user-defined) patterns in local variables.
When performing an optimized build, and when possible, we don't do store but inline them directly.

For soundness, SI-5158, SI-6070, we must always store the values of mutable case class fields.

(Specifying -optimize is the only way to suppress emitting these local variables.
An unoptimized build will always generate them, which was deemed the right default during the meeting.)

(updated flags for t4425 to get consistent runs on optimized and non-optimized partest runs
 by always passing -optimize)
@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 8, 2012

fixed flags file so optimized and non-optimized invocations of partest behave consistently
(by adding -optimize to t4425.flags)

PLS REBUILD ALL

@scala-jenkins
Copy link

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

@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/830/

@scala-jenkins
Copy link

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

@scala-jenkins
Copy link

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

adriaanm pushed a commit that referenced this pull request Aug 8, 2012
SI-5739 (bis) vals for subpatterns when -g:patmatvars
@adriaanm adriaanm merged commit 5fc4057 into scala:2.10.x Aug 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants