Skip to content

Allow experimental language imports in experimental scopes #13396

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 0 additions & 4 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3114,10 +3114,6 @@ object Parsers {
languageImport(tree) match
case Some(prefix) =>
in.languageImportContext = in.languageImportContext.importContext(imp, NoSymbol)
if prefix == nme.experimental
&& selectors.exists(sel => Feature.experimental(sel.name) != Feature.scala2macros)
then
Feature.checkExperimentalFeature("features", imp.srcPos)
for
case ImportSelector(id @ Ident(imported), EmptyTree, _) <- selectors
if allSourceVersionNames.contains(imported)
Expand Down
26 changes: 26 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Decorators._
import Symbols._, SymUtils._, NameOps._
import ContextFunctionResults.annotateContextResults
import config.Printers.typr
import config.Feature
import reporting._

object PostTyper {
Expand Down Expand Up @@ -448,6 +449,31 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
throw ex
}

/** In addition to normal processing, check that experimental language
* imports are done only in experimental scopes, or in scopes where
* all statements are @experimental definitions.
*/
override def transformStats(trees: List[Tree], exprOwner: Symbol)(using Context): List[Tree] =

def onlyExperimentalDefs = trees.forall {
case _: Import | EmptyTree => true
case stat: MemberDef => stat.symbol.isExperimental
case _ => false
}

def checkExperimentalImports =
for case imp @ Import(qual, selectors) <- trees do
languageImport(qual) match
case Some(nme.experimental)
if !ctx.owner.isInExperimentalScope && !onlyExperimentalDefs
&& selectors.exists(sel => Feature.experimental(sel.name) != Feature.scala2macros) =>
Feature.checkExperimentalFeature("features", imp.srcPos)
case _ =>

try super.transformStats(trees, exprOwner)
finally checkExperimentalImports
end transformStats

/** Transforms the rhs tree into a its default tree if it is in an `erased` val/def.
* Performed to shrink the tree that is known to be erased later.
*/
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class CompilationTests {
compileFile("tests/neg-custom-args/matchable.scala", defaultOptions.and("-Xfatal-warnings", "-source", "future")),
compileFile("tests/neg-custom-args/i7314.scala", defaultOptions.and("-Xfatal-warnings", "-source", "future")),
compileFile("tests/neg-custom-args/feature-shadowing.scala", defaultOptions.and("-Xfatal-warnings", "-feature")),
compileDir("tests/neg-custom-args/hidden-type-errors", defaultOptions.and("-explain")),
compileDir("tests/neg-custom-args/hidden-type-errors", defaultOptions.and("-explain")),
).checkExpectedErrors()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import language.experimental.erasedDefinitions // error
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See pos-curstomArgs/experimental.erased for the same example that compiles. It's not CanThrow that triggers the error but other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested pos-curstomArgs/experimental.erased manually with -Yno-experimental. I assume that will be tested anyway since we start with a stable compiler. Or do we need a separate pos test that asserts -Yno-experimental`?

Copy link
Contributor

@nicolasstucki nicolasstucki Aug 26, 2021

Choose a reason for hiding this comment

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

That is the same behavior as in master.

import language.experimental.erasedDefinitions
import annotation.experimental

@experimental
erased class CanThrow[-E <: Exception]

Compiles with snapshot build without error but fails when we add -Yno-experimental.

If we cannot compile it with -Yno-experimental we will not be able to re-bootstrap on a stable compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested it manually with -Yno-experimental and verified that it compiles

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see what goes on. The generated package object is not experimental.

Copy link
Contributor

Choose a reason for hiding this comment

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

The error message of is also unhelpful. If a user does this change they will have no idea why it started failing.

import language.experimental.erasedDefinitions // error
import annotation.experimental

@experimental
erased class CanThrow[-E <: Exception]
+ object Foo

The scope of CanThrow is still experimental. But now it it says Experimental features may only be used with a nightly or snapshot version of the compiler which has not changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now disregard synthetic definitions in the check. That also takes care of compiler-generated companion objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

import annotation.experimental

@experimental
erased class CanThrow[-E <: Exception]

def other = 1
6 changes: 0 additions & 6 deletions tests/neg-custom-args/no-experimental/experimental.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,4 @@ class Test2 {

class Test6 {
import scala.language.experimental // ok
}

class Test7 {
import scala.language.experimental
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted since we now check experimental imports after typer, and Test7 produces errors at Typer

import experimental.genericNumberLiterals // error: no aliases can be used to refer to a language import
val x: BigInt = 13232202002020202020202 // error
}
10 changes: 10 additions & 0 deletions tests/pos/experimental-erased-2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import language.experimental.erasedDefinitions
import annotation.experimental

@experimental object Test:

erased class CanThrow[-E <: Exception]

def other = 1


6 changes: 6 additions & 0 deletions tests/pos/experimental-erased.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import language.experimental.erasedDefinitions
import annotation.experimental

@experimental
erased class CanThrow[-E <: Exception]