Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a6c1451

Browse files
authoredOct 16, 2021
Merge pull request #4556 from gzm0/ec-warn
Fix #4129: Warn if the default global execution context is used.
2 parents 0b0eefe + a380c7f commit a6c1451

File tree

21 files changed

+295
-6
lines changed

21 files changed

+295
-6
lines changed
 

‎compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala

+5
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ trait JSDefinitions {
135135

136136
lazy val EnableReflectiveInstantiationAnnotation = getRequiredClass("scala.scalajs.reflect.annotation.EnableReflectiveInstantiation")
137137

138+
lazy val ExecutionContextModule = getRequiredModule("scala.concurrent.ExecutionContext")
139+
lazy val ExecutionContext_global = getMemberMethod(ExecutionContextModule, newTermName("global"))
140+
141+
lazy val ExecutionContextImplicitsModule = getRequiredModule("scala.concurrent.ExecutionContext.Implicits")
142+
lazy val ExecutionContextImplicits_global = getMemberMethod(ExecutionContextImplicitsModule, newTermName("global"))
138143
}
139144

140145
// scalastyle:on line.size.limit

‎compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala

+28
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,34 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
377377
|program is unlikely to function properly.""".stripMargin)
378378
super.transform(tree)
379379

380+
case tree if tree.symbol == ExecutionContext_global ||
381+
tree.symbol == ExecutionContextImplicits_global =>
382+
if (scalaJSOpts.warnGlobalExecutionContext) {
383+
global.runReporting.warning(tree.pos,
384+
"""The global execution context in Scala.js is based on JS Promises (microtasks).
385+
|Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
386+
|
387+
|Unfortunately, there is no way with ECMAScript only to implement a performant
388+
|macrotask execution context (and hence Scala.js core does not contain one).
389+
|
390+
|We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
391+
|Please refer to the README.md of that project for more details regarding
392+
|microtask vs. macrotask execution contexts.
393+
|
394+
|If you do not care about macrotask fairness, you can silence this warning by:
395+
|- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
396+
|- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
397+
|- Using scala.scalajs.concurrent.JSExecutionContext.queue
398+
| (the implementation of ExecutionContext.global in Scala.js) directly.
399+
|
400+
|If you do not care about performance, you can use
401+
|scala.scalajs.concurrent.QueueExecutionContext.timeouts().
402+
|It is based on setTimeout which makes it fair but slow (due to clamping).
403+
""".stripMargin,
404+
WarningCategory.Other, tree.symbol)
405+
}
406+
super.transform(tree)
407+
380408
// Validate js.constructorOf[T]
381409
case TypeApply(ctorOfTree, List(tpeArg))
382410
if ctorOfTree.symbol == JSPackage_constructorOf =>

‎compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSOptions.scala

+5
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ trait ScalaJSOptions {
3838
* should they be mapped to)? */
3939
def sourceURIMaps: List[URIMap]
4040

41+
/** Whether to warn if the global execution context is used.
42+
*
43+
* See the warning itself or #4129 for context.
44+
*/
45+
def warnGlobalExecutionContext: Boolean
4146
}
4247

4348
object ScalaJSOptions {

‎compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSPlugin.scala

+3
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
6262
else
6363
relSourceMap.toList.map(URIMap(_, absSourceMap))
6464
}
65+
var warnGlobalExecutionContext: Boolean = true
6566
var _sourceURIMaps: List[URIMap] = Nil
6667
var relSourceMap: Option[URI] = None
6768
var absSourceMap: Option[URI] = None
@@ -121,6 +122,8 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
121122
fixClassOf = true
122123
} else if (option == "genStaticForwardersForNonTopLevelObjects") {
123124
genStaticForwardersForNonTopLevelObjects = true
125+
} else if (option == "nowarnGlobalExecutionContext") {
126+
warnGlobalExecutionContext = false
124127
} else if (option.startsWith("mapSourceURI:")) {
125128
val uris = option.stripPrefix("mapSourceURI:").split("->")
126129

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Scala.js (https://www.scala-js.org/)
3+
*
4+
* Copyright EPFL.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (https://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package org.scalajs.nscplugin.test
14+
15+
import org.scalajs.nscplugin.test.util._
16+
import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn
17+
18+
import org.junit.Assume._
19+
import org.junit.Test
20+
21+
class GlobalExecutionContextNoWarnTest extends DirectTest with TestHelpers {
22+
23+
override def extraArgs: List[String] =
24+
super.extraArgs ::: List("-P:scalajs:nowarnGlobalExecutionContext")
25+
26+
@Test
27+
def noWarnOnUsage: Unit = {
28+
"""
29+
import scala.concurrent.ExecutionContext.global
30+
31+
object Enclosing {
32+
global
33+
}
34+
""".hasNoWarns()
35+
}
36+
37+
@Test
38+
def noWarnOnImplicitUsage: Unit = {
39+
"""
40+
import scala.concurrent.ExecutionContext.Implicits.global
41+
42+
object Enclosing {
43+
scala.concurrent.Future { }
44+
}
45+
""".hasNoWarns()
46+
}
47+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Scala.js (https://www.scala-js.org/)
3+
*
4+
* Copyright EPFL.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (https://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package org.scalajs.nscplugin.test
14+
15+
import org.scalajs.nscplugin.test.util._
16+
import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn
17+
18+
import org.junit.Assume._
19+
import org.junit.Test
20+
21+
class GlobalExecutionContextWarnTest extends DirectTest with TestHelpers {
22+
23+
@Test
24+
def warnOnUsage: Unit = {
25+
"""
26+
import scala.concurrent.ExecutionContext.global
27+
28+
object Enclosing {
29+
global
30+
}
31+
""" hasWarns
32+
"""
33+
|newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks).
34+
|Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
35+
|
36+
|Unfortunately, there is no way with ECMAScript only to implement a performant
37+
|macrotask execution context (and hence Scala.js core does not contain one).
38+
|
39+
|We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
40+
|Please refer to the README.md of that project for more details regarding
41+
|microtask vs. macrotask execution contexts.
42+
|
43+
|If you do not care about macrotask fairness, you can silence this warning by:
44+
|- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
45+
|- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
46+
|- Using scala.scalajs.concurrent.JSExecutionContext.queue
47+
| (the implementation of ExecutionContext.global in Scala.js) directly.
48+
|
49+
|If you do not care about performance, you can use
50+
|scala.scalajs.concurrent.QueueExecutionContext.timeouts().
51+
|It is based on setTimeout which makes it fair but slow (due to clamping).
52+
|
53+
| global
54+
| ^
55+
"""
56+
}
57+
58+
@Test
59+
def warnOnImplicitUsage: Unit = {
60+
"""
61+
import scala.concurrent.ExecutionContext.Implicits.global
62+
63+
object Enclosing {
64+
scala.concurrent.Future { }
65+
}
66+
""" hasWarns
67+
"""
68+
|newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks).
69+
|Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
70+
|
71+
|Unfortunately, there is no way with ECMAScript only to implement a performant
72+
|macrotask execution context (and hence Scala.js core does not contain one).
73+
|
74+
|We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
75+
|Please refer to the README.md of that project for more details regarding
76+
|microtask vs. macrotask execution contexts.
77+
|
78+
|If you do not care about macrotask fairness, you can silence this warning by:
79+
|- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
80+
|- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
81+
|- Using scala.scalajs.concurrent.JSExecutionContext.queue
82+
| (the implementation of ExecutionContext.global in Scala.js) directly.
83+
|
84+
|If you do not care about performance, you can use
85+
|scala.scalajs.concurrent.QueueExecutionContext.timeouts().
86+
|It is based on setTimeout which makes it fair but slow (due to clamping).
87+
|
88+
| scala.concurrent.Future { }
89+
| ^
90+
"""
91+
}
92+
93+
@Test
94+
def noWarnIfSelectivelyDisabled: Unit = {
95+
assumeTrue(scalaSupportsNoWarn)
96+
97+
"""
98+
import scala.annotation.nowarn
99+
import scala.concurrent.ExecutionContext.global
100+
101+
object Enclosing {
102+
global: @nowarn("cat=other")
103+
}
104+
""".hasNoWarns()
105+
}
106+
107+
@Test
108+
def noWarnQueue: Unit = {
109+
/* Test that JSExecutionContext.queue does not warn for good measure.
110+
* We explicitly say it doesn't so we want to notice if it does.
111+
*/
112+
113+
"""
114+
import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
115+
116+
object Enclosing {
117+
scala.concurrent.Future { }
118+
}
119+
""".hasNoWarns()
120+
}
121+
122+
}

‎ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import java.util.concurrent.ConcurrentHashMap
1717
import scala.util.matching.Regex
1818

1919
object ScalaJSVersions extends VersionChecks(
20-
current = "1.7.2-SNAPSHOT",
20+
current = "1.8.0-SNAPSHOT",
2121
binaryEmitted = "1.7"
2222
)
2323

‎junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala

+7-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,13 @@
1313
package org.scalajs.junit
1414

1515
import scala.concurrent.Future
16-
import scala.concurrent.ExecutionContext.Implicits.global
16+
17+
/* Use the queue execution context (based on JS promises) explicitly:
18+
* We do not have anything better at our disposal and it is accceptable in
19+
* terms of fairness: All we use it for is to map over a completed Future once.
20+
*/
21+
import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
22+
1723
import scala.util.{Try, Success}
1824

1925
package object async {

‎junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala

+8-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,14 @@
1313
package org.scalajs.junit
1414

1515
import scala.concurrent.Future
16-
import scala.concurrent.ExecutionContext.Implicits.global
16+
17+
/* Use the queue execution context (based on JS promises) explicitly:
18+
* We do not have anything better at our disposal and it is accceptable in
19+
* terms of fairness: We only use it for test dispatching and orchestation.
20+
* The real async work is done in Bootstrapper#invokeTest which does not take
21+
* an (implicit) ExecutionContext parameter.
22+
*/
23+
import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
1724

1825
import scala.util.{Try, Success, Failure}
1926

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
error: Usage: -Yresolve-term-conflict:<strategy>
2+
where <strategy> choices are package, object, error (default: error)
3+
error: bad option: '-Yresolve-term-conflict'
4+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
5+
error: flags file may only contain compiler options, found: -Yresolve-term-conflict
6+
four errors found
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
error: bad option: '-badCompilerFlag'
2+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -Yopt:badChoice
3+
error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -Yopt:badChoice
4+
three errors found
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
error: Usage: -Yresolve-term-conflict:<strategy> where <strategy> choices are package, object, error (default: error).
2+
error: bad option: '-Yresolve-term-conflict'
3+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
4+
error: flags file may only contain compiler options, found: -Yresolve-term-conflict
5+
four errors found
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
error: bad option: '-badCompilerFlag'
2+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -opt:badChoice
3+
error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -opt:badChoice
4+
three errors found
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
error: Usage: -Yresolve-term-conflict:<strategy> where <strategy> choices are package, object, error (default: error).
2+
error: bad option: '-Yresolve-term-conflict'
3+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
4+
error: flags file may only contain compiler options, found: -Yresolve-term-conflict
5+
4 errors
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
error: bad option: '-badCompilerFlag'
2+
error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -opt:badChoice
3+
error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -opt:badChoice
4+
3 errors

‎partest/src/main/scala-new-partest/scala/tools/partest/scalajs/ScalaJSRunner.scala

+8
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
package scala.tools.partest.scalajs
1414

15+
import java.io.File
16+
1517
import scala.tools.partest.nest
1618
import scala.tools.partest.nest.{AbstractRunner, DirectCompiler, TestInfo}
1719

@@ -23,6 +25,12 @@ class ScalaJSRunner(testInfo: ScalaJSTestInfo, suiteRunner: AbstractRunner,
2325
new DirectCompiler(this) with ScalaJSDirectCompiler
2426
}
2527

28+
override def flagsForCompilation(sources: List[File]): List[String] = {
29+
// Never warn, so we do not need to update tons of checkfiles.
30+
"-P:scalajs:nowarnGlobalExecutionContext" ::
31+
super.flagsForCompilation(sources)
32+
}
33+
2634
override def extraJavaOptions = {
2735
super.extraJavaOptions ++ Seq(
2836
s"-Dscalajs.partest.optMode=${options.optMode.id}",

‎partest/src/main/scala-old-partest/scala/tools/partest/scalajs/ScalaJSRunner.scala

+7
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ class ScalaJSRunner(testFile: File, suiteRunner: SuiteRunner,
4747
}
4848

4949
override def newCompiler = new DirectCompiler(this) with ScalaJSDirectCompiler
50+
51+
override def flagsForCompilation(sources: List[File]): List[String] = {
52+
// Never warn, so we do not need to update tons of checkfiles.
53+
"-P:scalajs:nowarnGlobalExecutionContext" ::
54+
super.flagsForCompilation(sources)
55+
}
56+
5057
override def extraJavaOptions = {
5158
super.extraJavaOptions ++ Seq(
5259
s"-Dscalajs.partest.optMode=${options.optMode.id}",
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.