Skip to content

Consider signatures of methods before and after erasure in ExtractAPI#2261

Merged
eed3si9n merged 12 commits intosbt:0.13from
Duhemm:fix-1171
Dec 16, 2015
Merged

Consider signatures of methods before and after erasure in ExtractAPI#2261
eed3si9n merged 12 commits intosbt:0.13from
Duhemm:fix-1171

Conversation

@Duhemm
Copy link
Contributor

@Duhemm Duhemm commented Nov 4, 2015

The signatures of methods that have value classes as arguments or return
type change during the erasure phase. Because we only registered
signatures before the erasure, we missed some API changes when a class
was changed to a value class (or a value class changed to a class).

This commit fixes this problem by recording the signatures of method
before and after erasure.

Fixes #1171

The signatures of methods that have value classes as arguments or return
type change during the erasure phase. Because we only registered
signatures before the erasure, we missed some API changes when a class
was changed to a value class (or a value class changed to a class).

This commit fixes this problem by recording the signatures of method
before and after erasure.

Fixes sbt#1171
@typesafe-tools
Copy link

Can one of the admins verify this patch?

@eed3si9n
Copy link
Member

@Duhemm Could you fix the codacy failure due to the lack of return type signature on public method, plz?

eed3si9n added a commit that referenced this pull request Dec 16, 2015
Consider signatures of methods before and after erasure in ExtractAPI
@eed3si9n eed3si9n merged commit d3cb469 into sbt:0.13 Dec 16, 2015
@dwijnand
Copy link
Member

Very cool. Nice to see old, old bugs get fixes. Well done.

adriaanm added a commit to adriaanm/sbt that referenced this pull request Dec 20, 2015
adriaanm added a commit to adriaanm/sbt that referenced this pull request Dec 20, 2015
adriaanm added a commit to adriaanm/sbt that referenced this pull request Dec 21, 2015
adriaanm added a commit to adriaanm/sbt that referenced this pull request Dec 21, 2015
eed3si9n referenced this pull request in adriaanm/sbt Dec 31, 2015
For refinement types, the Structure was already restricted
to declarations (and not inherited members), but all base types
were still included for a refinement's parents, which would
create unwieldy, and even erroneous (cyclic) types by expanding
all constituents of an intersection type to add all base types.

Since the logic already disregarded inherited members, it seems
logical to only include direct parents, and not all ancestor types.

Incremental compilation no longer includes a huge cyclic type,
and thus `ShowAPI` in principle does not need to deal with cycles,
but we'll keep that functionality in there for now.

```
class Dep {
  def bla(c: Boolean) = if (c) new Value else "bla"
}

class Value extends java.lang.Comparable[Value] { def compareTo(that: Value): Int = 1 }
```
@eed3si9n
Copy link
Member

eed3si9n commented Jan 7, 2016

This needs to be forward ported.

@gkossakowski
Copy link
Contributor

//cc @Duhemm

@Duhemm
Copy link
Contributor Author

Duhemm commented Feb 2, 2016

@gkossakowski I didn't. I think that the increase of the size of the API has been somewhat mitigated by #2414. I'll try to look into it.

@gkossakowski
Copy link
Contributor

Oh, I didn't see #2414. It will indeed help with API structure size.

Do you need to detect whether a type in a signature just changed from a value class to a regular class or do you need the full information about what it erases to?

@smarter
Copy link
Contributor

smarter commented Feb 2, 2016

It's still bigger than it needs to be, consider:

class A(val self: Int) extends AnyVal
class B {
  def foo(a: A): A = new A(1)
}

Then ExtractAPI returns four signatures for foo but we only need twos (non-erased and fully-erased):

def foo(a: this#A): this#A
def foo(a: this#A): scala.this#Int
def foo(a: scala.this#Int): this#A
def foo(a: scala.this#Int): scala.this#Int

@gkossakowski
Copy link
Contributor

Would it be enough to introduce a special TypeRef that is used to referring to value classes and then return just one signature of foo? The switch from AnyVal to AnyRef would be detected by change from ValueClassTypeRef to TypeRef.

@smarter
Copy link
Contributor

smarter commented Feb 2, 2016

I think you still need to know what the underlying type is, consider three files A.scala, B.scala, C.scala:

class A(val self: Int) extends AnyVal

object B {
  def foo: A = new A(1)
}

object C {
  def main(args: Array[String]): Unit = {
    B.foo
  }
}

If we change the underlying type of A from Int to Double, then B.scala will be recompiled. But if you don't register that the underlying type of A has changed, then C.scala won't be recompiled and will fail at runtime, because the signature of B.foo in C.class will be incorrect.

@gkossakowski
Copy link
Contributor

Yes, you're right. However, the issue is that there're two different ways to refer to A (before erasure and after erasure) so why can't we encode that in ValueClassTypeRef that would contain reference to unerased and erased type? This way the rest of api extraction can remain oblivious to value classes.

@smarter
Copy link
Contributor

smarter commented Feb 2, 2016

why can't we encode that in ValueClassTypeRef that would contain reference to unerased and erased type

That seems like a nice idea, though instead of having a reference to the erased type, I'd like to have a reference to the underlying type. In Dotty, a value class can have another value class as its underlying type.

@Duhemm Duhemm deleted the fix-1171 branch February 8, 2016 08:29
smarter added a commit to smarter/zinc that referenced this pull request Apr 9, 2016
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to sbt#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that use a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/zinc that referenced this pull request Apr 9, 2016
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to sbt#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that use a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/zinc that referenced this pull request Apr 9, 2016
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to sbt#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 14, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 14, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 15, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 15, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 15, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 15, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 16, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 18, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 19, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 20, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
smarter added a commit to smarter/sbt that referenced this pull request Apr 20, 2016
This is a backport of sbt/zinc#95

The previous approach to value class API (introduced by sbt#2261 and
refined by sbt#2413 and sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced
recompilations.
This is no longer necessary thanks to sbt#2523: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of `<init>` changes and since every class uses the name
`<init>`, we don't need to do anything special to trigger recompilations
either.
eed3si9n pushed a commit to eed3si9n/scala that referenced this pull request May 14, 2019
eed3si9n pushed a commit to eed3si9n/scala that referenced this pull request May 14, 2019
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to scala#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.
lrytz pushed a commit to lrytz/scala that referenced this pull request Nov 5, 2019
lrytz pushed a commit to lrytz/scala that referenced this pull request Nov 5, 2019
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to scala#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.

Rewritten from sbt/zinc@1e7e99e
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.

7 participants