Fix #97: Avoid spurious recompilations when unrelated constructor changes #288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smarter
force-pushed
the
fix/overcompile-constructor
branch
7 times, most recently
from
044e71b to
1bd0158
Compare
jvican
approved these changes
Apr 7, 2017
eed3si9n
approved these changes
Apr 7, 2017
|
@smarter Give me until next Monday to think about potential interactions, and we merge 😉. I just want to be sure. |
|
Ping for review. |
|
I already gave my 👍 so we're waiting on @jvican? |
|
This change looks great, I've confirmed this will not have a bad interaction with anything. I was worried about the format representation and wanted to double check that it was unique. I am resolving the conflicts caused by the formatting PR manually, and when the light is green I'm merging. |
jvican
force-pushed
the
fix/overcompile-constructor
branch
from
1bd0158 to
a3f26d8
Compare
|
Travis was blocked, that's why I closed and opened again. I'll merge this when it's green. |
jvican
force-pushed
the
fix/overcompile-constructor
branch
from
a3f26d8 to
60d7978
Compare
…changes The name hashing algorithm is designed to take implicit conversions into account: when a name "foo" is changed somewhere in a dependency of X, you have to recompile X if it uses the name "foo", even if the usage of "foo" in X is completely unrelated, just because this might have an effect on available implicit conversions. However, there is one case where we can be sure that implicit conversions will not kick in: when we call a constructor. A constructor name is always "<init>", this PR now replaces this name by "pkgA;pkgB;className;init;", this mean that we no longer recompile classes when an unrelated constructor in a used class changed (see the new test `constructors-unrelated` for an example).
jvican
force-pushed
the
fix/overcompile-constructor
branch
from
60d7978 to
aca8dfa
Compare
allanrenucci
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 13, 2018
Use a different encoding for constructor: constructor.enclosingClass.fullName ++ ";init;"
eed3si9n
pushed a commit
to eed3si9n/scala
that referenced
this pull request
May 14, 2019
Use a different encoding for constructor: constructor.enclosingClass.fullName ++ ";init;"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The name hashing algorithm is designed to take implicit conversions into
account: when a name "foo" is changed somewhere in a dependency of X,
you have to recompile X if it uses the name "foo", even if the usage of
"foo" in X is completely unrelated, just because this might have an
effect on available implicit conversions. However, there is one case
where we can be sure that implicit conversions will not kick in: when we
call a constructor. A constructor name is always "<init>", this PR now
replaces this name by "pkgA;pkgB;className;init;", this mean that we no
longer recompile classes when an unrelated constructor in a used class
changed (see the new test
constructors-unrelatedfor an example).