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

allow lazy loading of parts of types #1

Closed
adriaanm opened this issue Dec 20, 2012 · 0 comments
Closed

allow lazy loading of parts of types #1

adriaanm opened this issue Dec 20, 2012 · 0 comments

Comments

@adriaanm
Copy link
Contributor

similar to using TypeCompleters to defer loading the info of a Symbol,
it would be useful to be able to defer loading a PolyType's result type,
so that we can accurately find out its type parameters without forcing the result type (which may cause cycles / be too costly)

magarciaEPFL added a commit that referenced this issue Feb 14, 2014
context for ClassBytecodeWriter
odersky added a commit to odersky/dotty that referenced this issue Jun 7, 2014
-Ycheck:front fails in collections.scala because of the bug exhibited
by pending/pos/refinedSubtyping.scala.

Also, there's a problem in TypeComparers.scala which is minimized
in pending/pos/test.scala.

Otherwise all tests pass.

This commit establishes a baseline for further experimentation.
odersky added a commit that referenced this issue Oct 6, 2015
OlivierBlanvillain referenced this issue in OlivierBlanvillain/dotty Dec 8, 2016
This change allows an additional notation of the @throws annotation:

  Old-style: @throws(classOf[Exception])
  New-style: @throws[Exception]

The optional String argument moves @throws in line with @deprecated,
@migration, etc. and prevents confusion caused by the default inheritance
of ScalaDoc comments and the non-inheritance of annotations.

  Before: /** This method does ...
            * @throws IllegalArgumentException if `a` is less than 0. */
          @throws(classOf[IllegalArgumentException])
          def foo(a: Int) = ...

  Now:    /** This method does ... */
          @throws[IllegalArgumentException]("if `a` is less than 0")
          def foo(a: Int) = ...

ScalaDoc @throws tags remain supported for cases where documentation of
thrown exceptions is needed, but are not supposed to be added to the
exception attribute of the class file.

In this commit the necessary compiler support is added.
The code to extract exceptions from annotations is now shared instead
of being duplicated all over the place.

The change is completely source and binary compatible, except that the code
is now enforcing that the type thrown is a subtype of Throwable as mandated
by the JVM spec  instead of allowing something like @throws(classOf[String]).

Not in this commit:
  - ScalaDoc support to add the String argument to ScalaDoc's exception list
  - Adaption of the library
OlivierBlanvillain referenced this issue in OlivierBlanvillain/dotty Dec 8, 2016
[Parts of this patch and some of the commentary are from @paulp]

This took me so long to figure out I can't even tell you. Partly because
there were two different bugs, one which only arose for trait forwarders
and one for mirror class forwarders, and every time I'd make one set
of tests work another set would start failing. The runtime failures
associated with these bugs were fairly well hidden because you usually
have to go through java to encounter them: scala doesn't pay that much
attention to generic signatures, so they can be wrong and scala might still
generate correct code. But java is not so lucky.

Bug #1)

During mixin composition, classes which extend traits receive forwarders
to the implementations. An attempt was made to give these the correct
info (in method "cloneBeforeErasure") but it was prone to giving
the wrong answer, because: the key attribute which the forwarder
must capture is what the underlying method will erase to *where the
implementation is*, not how it appears to the class which contains it.
That means the signature of the forwarder must be no more precise than
the signature of the inherited implementation unless additional measures
will be taken.

This subtle difference will put on an unsubtle show for you in test
run/t3452.scala.

    trait C[T]
    trait Search[M] { def search(input: M): C[Int] = null }
    object StringSearch extends Search[String] { }
    StringSearch.search("test");  // java
    // java.lang.NoSuchMethodError: StringSearch.search(Ljava/lang/String;)LC;

The principled thing to do here would be to create a pair of
methods in the host class: a mixin forwarder with the erased
signature `(String)C[Int]`, and a bridge method with the same
erased signature as the trait interface facet.

But, this turns out to be pretty hard to retrofit onto the
current setup of Mixin and Erasure, mostly due to the fact
that mixin happens after erasure which has already taken
care of bridging.

For a future, release, we should try to move all bridging
after mixin, and pursue this approach. But for now, what can
we do about `LinkageError`s for Java clients?

This commit simply checks if the pre-erasure method signature
that we generate for the trait forward erases identically to
that of the interface method. If so, we can be precise. If not,
we emit the erased signature as the generic signature.

Bug scala#2) The same principle is at work, at a different location.
During genjvm, objects without declared companion classes
are given static forwarders in the corresponding class, e.g.

    object Foo { def bar = 5 }

which creates these classes (taking minor liberties):

    class Foo$ { static val MODULE$ = new Foo$ ; def bar = 5 }
    class Foo  { static def bar = Foo$.MODULE$.bar }

In generating these, genjvm circumvented the usual process whereby one
creates a symbol and gives it an info, preferring to target the bytecode
directly. However generic signatures are calculated from symbol info
(in this case reusing the info from the module class.) Lacking even the
attempt which was being made in mixin to "clone before erasure", we
would have runtime failures of this kind:

    abstract class Foo {
      type T
      def f(x: T): List[T] = List()
    }
    object Bar extends Foo { type T = String }
    Bar.f("");    // java
    // java.lang.NoSuchMethodError: Bar.f(Ljava/lang/String;)Lscala/collection/immutable/List;

Before/after this commit:

    <   signature                                     f  (Ljava/lang/String;)Lscala/collection/immutable/List<Ljava/lang/String;>;
    ---
    >   signature                                     f  (Ljava/lang/Object;)Lscala/collection/immutable/List<Ljava/lang/Object;>;

This takes the warning count for compiling collections under
`-Ycheck:jvm` from 1521 to 26.
OlivierBlanvillain referenced this issue in OlivierBlanvillain/dotty Dec 12, 2016
This change allows an additional notation of the @throws annotation:

  Old-style: @throws(classOf[Exception])
  New-style: @throws[Exception]

The optional String argument moves @throws in line with @deprecated,
@migration, etc. and prevents confusion caused by the default inheritance
of ScalaDoc comments and the non-inheritance of annotations.

  Before: /** This method does ...
            * @throws IllegalArgumentException if `a` is less than 0. */
          @throws(classOf[IllegalArgumentException])
          def foo(a: Int) = ...

  Now:    /** This method does ... */
          @throws[IllegalArgumentException]("if `a` is less than 0")
          def foo(a: Int) = ...

ScalaDoc @throws tags remain supported for cases where documentation of
thrown exceptions is needed, but are not supposed to be added to the
exception attribute of the class file.

In this commit the necessary compiler support is added.
The code to extract exceptions from annotations is now shared instead
of being duplicated all over the place.

The change is completely source and binary compatible, except that the code
is now enforcing that the type thrown is a subtype of Throwable as mandated
by the JVM spec  instead of allowing something like @throws(classOf[String]).

Not in this commit:
  - ScalaDoc support to add the String argument to ScalaDoc's exception list
  - Adaption of the library
OlivierBlanvillain referenced this issue in OlivierBlanvillain/dotty Dec 12, 2016
[Parts of this patch and some of the commentary are from @paulp]

This took me so long to figure out I can't even tell you. Partly because
there were two different bugs, one which only arose for trait forwarders
and one for mirror class forwarders, and every time I'd make one set
of tests work another set would start failing. The runtime failures
associated with these bugs were fairly well hidden because you usually
have to go through java to encounter them: scala doesn't pay that much
attention to generic signatures, so they can be wrong and scala might still
generate correct code. But java is not so lucky.

Bug #1)

During mixin composition, classes which extend traits receive forwarders
to the implementations. An attempt was made to give these the correct
info (in method "cloneBeforeErasure") but it was prone to giving
the wrong answer, because: the key attribute which the forwarder
must capture is what the underlying method will erase to *where the
implementation is*, not how it appears to the class which contains it.
That means the signature of the forwarder must be no more precise than
the signature of the inherited implementation unless additional measures
will be taken.

This subtle difference will put on an unsubtle show for you in test
run/t3452.scala.

    trait C[T]
    trait Search[M] { def search(input: M): C[Int] = null }
    object StringSearch extends Search[String] { }
    StringSearch.search("test");  // java
    // java.lang.NoSuchMethodError: StringSearch.search(Ljava/lang/String;)LC;

The principled thing to do here would be to create a pair of
methods in the host class: a mixin forwarder with the erased
signature `(String)C[Int]`, and a bridge method with the same
erased signature as the trait interface facet.

But, this turns out to be pretty hard to retrofit onto the
current setup of Mixin and Erasure, mostly due to the fact
that mixin happens after erasure which has already taken
care of bridging.

For a future, release, we should try to move all bridging
after mixin, and pursue this approach. But for now, what can
we do about `LinkageError`s for Java clients?

This commit simply checks if the pre-erasure method signature
that we generate for the trait forward erases identically to
that of the interface method. If so, we can be precise. If not,
we emit the erased signature as the generic signature.

Bug scala#2) The same principle is at work, at a different location.
During genjvm, objects without declared companion classes
are given static forwarders in the corresponding class, e.g.

    object Foo { def bar = 5 }

which creates these classes (taking minor liberties):

    class Foo$ { static val MODULE$ = new Foo$ ; def bar = 5 }
    class Foo  { static def bar = Foo$.MODULE$.bar }

In generating these, genjvm circumvented the usual process whereby one
creates a symbol and gives it an info, preferring to target the bytecode
directly. However generic signatures are calculated from symbol info
(in this case reusing the info from the module class.) Lacking even the
attempt which was being made in mixin to "clone before erasure", we
would have runtime failures of this kind:

    abstract class Foo {
      type T
      def f(x: T): List[T] = List()
    }
    object Bar extends Foo { type T = String }
    Bar.f("");    // java
    // java.lang.NoSuchMethodError: Bar.f(Ljava/lang/String;)Lscala/collection/immutable/List;

Before/after this commit:

    <   signature                                     f  (Ljava/lang/String;)Lscala/collection/immutable/List<Ljava/lang/String;>;
    ---
    >   signature                                     f  (Ljava/lang/Object;)Lscala/collection/immutable/List<Ljava/lang/Object;>;

This takes the warning count for compiling collections under
`-Ycheck:jvm` from 1521 to 26.
liufengyun pushed a commit that referenced this issue Oct 16, 2020
* Use new links 

Right now, there are multiple features with links that just say "The contents of this page have moved." It'd be more convenient if the links directly went to the updated pages.

* Delete implicit-function-types-spec.md

* Delete implicit-function-types.md

* Delete delegates.md

* Delete given-clauses.md

* Update features-classification.md
liufengyun added a commit that referenced this issue Oct 16, 2020
Remove deprecated docs and links to them (#1)
smarter pushed a commit to smarter/dotty that referenced this issue Apr 28, 2023
odersky added a commit that referenced this issue Dec 23, 2023
`given ... with` or `given ... = new { ... }` kinds of definitions now follow
the old rules. This allows recursive `given...with` definitions as they are
found in protoQuill.

We still have the old check in a later phase against directly recursive methods.
Of the three loops in the original i15474 we now detect #2 and #3 with new new
restrictions. #1 slips through since it is a loop involving a `given...with` instance
of `Conversion`, but is caught later with the recursive method check.

Previously tests #1 and #3 were detected with the recursive methods check and #2 slipped
through altogether.

The new rules are enough for defining simple givens with `=` without fear of looping.
Kordyjan pushed a commit that referenced this issue Jan 18, 2024
`given ... with` or `given ... = new { ... }` kinds of definitions now follow
the old rules. This allows recursive `given...with` definitions as they are
found in protoQuill.

We still have the old check in a later phase against directly recursive methods.
Of the three loops in the original i15474 we now detect #2 and #3 with new new
restrictions. #1 slips through since it is a loop involving a `given...with` instance
of `Conversion`, but is caught later with the recursive method check.

Previously tests #1 and #3 were detected with the recursive methods check and #2 slipped
through altogether.

The new rules are enough for defining simple givens with `=` without fear of looping.
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

No branches or pull requests

1 participant