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

Have a per-run time budget for import suggestions #9167

Merged
merged 5 commits into from
Jun 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ import scala.util.control.NonFatal
/** A compiler run. Exports various methods to compile source files */
class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with ConstraintRunInfo {

/** Default timeout to stop looking for further implicit suggestions, in ms.
* This is usually for the first import suggestion; subsequent suggestions
* may get smaller timeouts. @see ImportSuggestions.reduceTimeBudget
*/
private var myImportSuggestionBudget: Int =
Int.MinValue // sentinel value; means whatever is set in command line option

def importSuggestionBudget =
if myImportSuggestionBudget == Int.MinValue then ictx.settings.XimportSuggestionTimeout.value
else myImportSuggestionBudget

def importSuggestionBudget_=(x: Int) =
myImportSuggestionBudget = x

/** If this variable is set to `true`, some core typer operations will
* return immediately. Currently these early abort operations are
* `Typer.typed` and `Implicits.typedImplicit`.
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class ScalaSettings extends Settings.SettingGroup {
val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.")
val XverifySignatures: Setting[Boolean] = BooleanSetting("-Xverify-signatures", "Verify generic signatures in generated bytecode.")
val XignoreScala2Macros: Setting[Boolean] = BooleanSetting("-Xignore-scala2-macros", "Ignore errors when compiling code that calls Scala2 macros, these will fail at runtime.")
val XimportSuggestionTimeout: Setting[Int] = IntSetting("-Ximport-suggestion-timeout", "Timeout (in ms) for searching for import suggestions when errors are reported.", 8000)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgodzik You may want to set this timeout to a very low or even zero value when using the compiler from metals.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint! I think we can set it to 0 now, since we don't use that information in the IDE currently.


val XmixinForceForwarders = ChoiceSetting(
name = "-Xmixin-force-forwarders",
Expand Down
20 changes: 15 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ trait ImportSuggestions:
/** Timeout to test a single implicit value as a suggestion, in ms */
private inline val testOneImplicitTimeOut = 500

/** Global timeout to stop looking for further implicit suggestions, in ms */
private inline val suggestImplicitTimeOut = 10000

/** A list of TermRefs referring to the roots where suggestions for
* imports of givens or extension methods that might fix a type error
* are searched.
Expand Down Expand Up @@ -145,7 +142,11 @@ trait ImportSuggestions:
*/
private def importSuggestions(pt: Type)(using Context): (List[TermRef], List[TermRef]) =
val timer = new Timer()
val deadLine = System.currentTimeMillis() + suggestImplicitTimeOut
val allotted = ctx.run.importSuggestionBudget
if allotted <= 1 then return (Nil, Nil)
implicits.println(i"looking for import suggestions, timeout = ${allotted}ms")
val start = System.currentTimeMillis()
val deadLine = start + allotted

// Candidates that are already available without explicit import because they
// are already provided by the context (imported or inherited) or because they
Expand Down Expand Up @@ -249,9 +250,18 @@ trait ImportSuggestions:
println("caught exception when searching for suggestions")
ex.printStackTrace()
(Nil, Nil)
finally timer.cancel()
finally
timer.cancel()
reduceTimeBudget(((System.currentTimeMillis() - start) min Int.MaxValue).toInt)
end importSuggestions

/** Reduce next timeout for import suggestions by the amount of time it took
* for current search, but but never less than to half of the previous budget.
*/
private def reduceTimeBudget(used: Int)(using Context) =
ctx.run.importSuggestionBudget =
(ctx.run.importSuggestionBudget - used) max (ctx.run.importSuggestionBudget / 2)

/** The `ref` parts of this list of pairs, discarding subsequent elements that
* have the same String part. Elements are sorted by their String parts.
*/
Expand Down
131 changes: 131 additions & 0 deletions tests/neg/i9160.scala

Large diffs are not rendered by default.

28 changes: 28 additions & 0 deletions tests/neg/missing-implicit-2.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-- Error: tests/neg/missing-implicit-2.scala:4:24 ----------------------------------------------------------------------
4 |val f = Future[Unit] { } // error
| ^
| Cannot find an implicit ExecutionContext. You might pass
| an (implicit ec: ExecutionContext) parameter to your method.
|
| The ExecutionContext is used to configure how and on which
| thread pools Futures will run, so the specific ExecutionContext
| that is selected is important.
|
| If your application does not define an ExecutionContext elsewhere,
| consider using Scala's global ExecutionContext by defining
| the following:
|
| implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
|
| The following import might fix the problem:
|
| import concurrent.ExecutionContext.Implicits.global
|
-- [E007] Type Mismatch Error: tests/neg/missing-implicit-2.scala:6:25 -------------------------------------------------
6 |val b: java.lang.Byte = (1: Byte) // error, but no hint
| ^^^^^^^
| Found: Byte
| Required: Byte²
|
| where: Byte is a class in package scala
| Byte² is a class in package java.lang
6 changes: 6 additions & 0 deletions tests/neg/missing-implicit-2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Predef.{byte2Byte => _, _}
import scala.concurrent.Future

val f = Future[Unit] { } // error

val b: java.lang.Byte = (1: Byte) // error, but no hint
25 changes: 25 additions & 0 deletions tests/neg/missing-implicit-3.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-- [E007] Type Mismatch Error: tests/neg/missing-implicit-3.scala:6:44 -------------------------------------------------
6 |val d: scala.concurrent.duration.Duration = (10, DAYS) // error
| ^^^^^^^^^^
| Found: (Int, java.util.concurrent.TimeUnit)
| Required: concurrent².duration.Duration
|
| where: concurrent is a package in package java.util
| concurrent² is a package in package scala
|
|
| The following import might fix the problem:
|
| import concurrent.duration.pairIntToDuration
|
-- [E008] Not Found Error: tests/neg/missing-implicit-3.scala:8:48 -----------------------------------------------------
8 |val d2: scala.concurrent.duration.Duration = 10.days // error
| ^^^^^^^
| value days is not a member of Int, but could be made available as an extension method.
|
| One of the following imports might fix the problem:
|
| import concurrent.duration.DurationInt
| import concurrent.duration.DurationLong
| import concurrent.duration.DurationDouble
|
8 changes: 8 additions & 0 deletions tests/neg/missing-implicit-3.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Predef.{byte2Byte => _, _}
import math.Numeric

val DAYS = scala.concurrent.duration.DAYS

val d: scala.concurrent.duration.Duration = (10, DAYS) // error

val d2: scala.concurrent.duration.Duration = 10.days // error
53 changes: 0 additions & 53 deletions tests/neg/missing-implicit.check
Original file line number Diff line number Diff line change
Expand Up @@ -17,56 +17,3 @@
|
| import math.Numeric.Implicits.infixNumericOps
|
-- Error: tests/neg/missing-implicit.scala:10:24 -----------------------------------------------------------------------
10 |val f = Future[Unit] { } // error
| ^
| Cannot find an implicit ExecutionContext. You might pass
| an (implicit ec: ExecutionContext) parameter to your method.
|
| The ExecutionContext is used to configure how and on which
| thread pools Futures will run, so the specific ExecutionContext
| that is selected is important.
|
| If your application does not define an ExecutionContext elsewhere,
| consider using Scala's global ExecutionContext by defining
| the following:
|
| implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
|
| The following import might fix the problem:
|
| import concurrent.ExecutionContext.Implicits.global
|
-- [E007] Type Mismatch Error: tests/neg/missing-implicit.scala:12:25 --------------------------------------------------
12 |val b: java.lang.Byte = (1: Byte) // error, but no hint
| ^^^^^^^
| Found: Byte
| Required: Byte²
|
| where: Byte is a class in package scala
| Byte² is a class in package java.lang
-- [E007] Type Mismatch Error: tests/neg/missing-implicit.scala:16:44 --------------------------------------------------
16 |val d: scala.concurrent.duration.Duration = (10, DAYS) // error
| ^^^^^^^^^^
| Found: (Int, java.util.concurrent.TimeUnit)
| Required: concurrent².duration.Duration
|
| where: concurrent is a package in package java.util
| concurrent² is a package in package scala
|
|
| The following import might fix the problem:
|
| import concurrent.duration.pairIntToDuration
|
-- [E008] Not Found Error: tests/neg/missing-implicit.scala:18:48 ------------------------------------------------------
18 |val d2: scala.concurrent.duration.Duration = 10.days // error
| ^^^^^^^
| value days is not a member of Int, but could be made available as an extension method.
|
| One of the following imports might fix the problem:
|
| import concurrent.duration.DurationInt
| import concurrent.duration.DurationLong
| import concurrent.duration.DurationDouble
|
12 changes: 0 additions & 12 deletions tests/neg/missing-implicit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,3 @@ import math.Numeric
def consume[T: Numeric](xs: List[T], limit: T): List[T] = xs match
case x :: xs1 if limit > 0 => consume(xs1, limit - x) // error // error
case _ => xs

import scala.concurrent.Future

val f = Future[Unit] { } // error

val b: java.lang.Byte = (1: Byte) // error, but no hint

val DAYS = scala.concurrent.duration.DAYS

val d: scala.concurrent.duration.Duration = (10, DAYS) // error

val d2: scala.concurrent.duration.Duration = 10.days // error