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

Should concrete inline methods be effectively final? #8564

Closed
odersky opened this issue Mar 17, 2020 · 6 comments
Closed

Should concrete inline methods be effectively final? #8564

odersky opened this issue Mar 17, 2020 · 6 comments

Comments

@odersky
Copy link
Contributor

odersky commented Mar 17, 2020

In #8543 we align static and dynamic semantics of inline methods and allow inline methods to implement abstract methods.

There is one exception where static and dynamic semantics still differ: inline methods can override other inline methods. Example:

class A:
  def f: Int
class B extends A:
  inline def f = 1
class C extends B:
  override inline def f = 2

val b: B = C()
b.f  // gives 1, since dispatched statically
val a: A = b
a.f  // gives 2, since dispatched dynamically

051039e tried to close this loophole by disallowing overriding of concrete inline methods. But it breaks ScalaTest. See the discussion in #8543 for why this is the case.

So, should we disallow this and require code that uses the pattern to be refactored? It might be cleaner in the end but could be painful to get there.

@nicolasstucki
Copy link
Contributor

Inline methods should be final as it is the only sound option.

@sjrd
Copy link
Member

sjrd commented Mar 18, 2020

To people downvoting this issue: please provide a concrete example/use case where the ability to override is exploited, so that we can propose alternatives and/or debate.

@neko-kai
Copy link
Contributor

There's the original usage in ScalaTest, which i believe is entirely acceptable, especially because you're unlikely to upcast this to access a different version of the method. Maybe we could consider an exemption for protected methods

@sjrd
Copy link
Member

sjrd commented Mar 18, 2020

For the ScalaTest use case, @smarter proposed two likely alternatives at #8543 (review). The first one being more hacky but with no refactoring needed, and the second one being a better overall design but requiring more refactoring.

@nicolasstucki
Copy link
Contributor

The ScalaTest use case can be encoded using @smarter suggestion in #8543 (review), performing the stattic dispatch in a single macro as in #8601 or defining the assert as an extension method on Assertions and another on Diagrams.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Mar 25, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Mar 25, 2020
@neko-kai
Copy link
Contributor

@nicolasstucki

defining the assert as an extension method on Assertions and another on Diagrams.

Extensions methods do not apply to implicit this, you'd be able to call assert only as this.assert if it's an extension method

odersky added a commit that referenced this issue Mar 31, 2020
Fix #8564: Make concrete inline methods final
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants