bug icon indicating copy to clipboard operation
bug copied to clipboard

warn on implicit def without explicit result type

Open scabug opened this issue 14 years ago • 9 comments

The following code snipet : https://gist.github.com/1427587 does not compile.

import java.util.Date

trait TDate 

trait TT[A1,T1]

trait TTFactory[F,G] {
  def create(f: F) : TT[F,G]
  def sample: F
}

object Impls {

  // If the c1 is declared before c2, it compiles fine
  // or if the implicit's result type is specified explicitly
  implicit def c2(s: Date)/* : TT[Date, TDate] */ = c1.create(s)  

  implicit val c1 = new TTFactory[Date,TDate] {
    def create(v: Date): TT[Date,TDate] = sys.error("")
    def sample = new Date
  }
}

scabug avatar Dec 03 '11 17:12 scabug

Imported From: https://issues.scala-lang.org/browse/SI-5265?orig=1 Reporter: maxime.levesque Affected Versions: 2.9.1 See #5348, #2206, #801

scabug avatar Dec 03 '11 17:12 scabug

@adriaanm said: see #2206, #801 and a plethora of duplicates

scabug avatar Jun 11 '12 13:06 scabug

@adriaanm said: warn unless enabled by feature flag

scabug avatar Jul 03 '12 15:07 scabug

@retronym said: I have a feeling there actually isn't a cycle in this example. Even if there is, we should report it.

Instead, I think the problem here is that typedBlock can trigger a type completion which causes the typedBlock for the same block to be reentered. Crucially, this happens after the DefTrees in the block have the symbols assigned. In the second run through typedBlock, the namer sees the attributed trees and decides not to enter them in scope.

The presentation compiler has a more robust namer to account for the sort of symptom, as caused by cancellation. I've moved that in to the regular namer in this WIP branch.

https://github.com/retronym/scala/tree/ticket/5265

That said, I definetely agree we should also have a warning / migration for unannotated implicits.

scabug avatar Oct 10 '14 07:10 scabug

@retronym said: That branch fails on (at least)

// t4716.scala
trait Bug2[ +A] extends TraversableOnce[A] {
  def ++[B >: A](that: TraversableOnce[B]) = {
    lazy val it = that.toIterator
    it
  }
}

I'm going to park this ticket again.

scabug avatar Oct 10 '14 07:10 scabug

This'd be a great project for somebody.

It has increased significance lately given that Scala 3 requires the explicit result type:

scala> implicit def x = 3
-- Error: ----------------------------------------------------------------------
1 |implicit def x = 3
  |             ^
  |           result type of implicit definition needs to be given explicitly
1 error found

So I would say that -Xlint should warn about this by default (since including explicit result types has been known at least since 2011 to be good practice), and -Xsource:3 should turn that warning into error.

Is there a volunteer who would like to tackle it?

SethTisue avatar Jul 19 '22 22:07 SethTisue

To whoever would like to take this on, note that the warning should not apply to local definitions, i.e., those for which sym.isLocalToBlock is true. Such definitions are only visible after they're defined, and hence doing cause problematic cycles. Early experimentation in dotty discovered that they had to be allowed to preserve reasonable code patterns.

sjrd avatar Jul 19 '22 23:07 sjrd

This'd be a great project for somebody.

Possible typo for Som, homebody. Or is that a portmanteau?

som-snytt avatar Jul 20 '22 18:07 som-snytt

Possible noise reduction if it accepted

implicit def f = foo.asInstanceOf[Bar]
implicit def g = foo: Bar

The first form, in particular, adds information. The second could be transposed.

The current exemption is for override in Scala 3/-Xsource:3.

som-snytt avatar Jul 21 '22 00:07 som-snytt